Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(hfbcat): Move get_doppler_shifted_freq from ephemeris #73

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

ketiltrout
Copy link
Member

@ketiltrout ketiltrout commented Jul 20, 2024

This isn't going to go with the rest of the ephemeris code to the new repository, so let's put is somewhere else.

Weirdly, the original docstring for this function seems to indicate it was meant to go in tools, but since its hardcoded to work work only on the HFBCatalog putting it in hfbcat directly seems most intuitive.

The re-constituted stub in ephermis avoids the circular import.

@ketiltrout ketiltrout requested review from ljgray and rikvl July 20, 2024 01:12
This isn't going to go with the rest of the ephemeris
code to the new repository, so let's put is somewhere else.

Weirdly, the original docstring for this function seems to
indicate it was meant to go in `tools`, but since its hardcoded
to work work only on the `HFBCatalog` putting it in `hfbcat`
directly seems most intuitive.

The re-constituted stub in `ephermis` avoids the circular import.

Changes to other files are from `black`.
Copy link
Contributor

@rikvl rikvl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I did a quick test and the function works as expected.

Thanks for fixing the docstring. I guess the function was developed in tools and I forgot to update the docstring when Richard recommended to put it in ephemeris.

I'm happy to leave get_range_rate in ephemeris.

@ketiltrout
Copy link
Member Author

Looks good. I did a quick test and the function works as expected.

Thanks for fixing the docstring. I guess the function was developed in tools and I forgot to update the docstring when Richard recommended to put it in ephemeris.

I figured it must have been something like that.

I'm happy to leave get_range_rate in ephemeris.

I am, too. It seems generic enough.

@ketiltrout ketiltrout merged commit 487532e into master Jul 21, 2024
4 checks passed
@ketiltrout ketiltrout deleted the move_get_doppler_shifted_freq branch July 21, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants