-
Notifications
You must be signed in to change notification settings - Fork 10
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
Weed out unused utils functions #381
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev_master #381 +/- ##
==============================================
+ Coverage 74.24% 74.94% +0.69%
==============================================
Files 56 56
Lines 7867 7770 -97
==============================================
- Hits 5841 5823 -18
+ Misses 2026 1947 -79 ☔ View full report in Codecov by Sentry. |
We have two incarnations of zenith distance to airmass (and vice-versa) conversion functions each. The implementations are (shockingly) identical, but the names and level of docstring details differ. Also, only one variant of each is actually used anywhere. However, I slightly prefer the name of the one that is not used and less well documented, mostly for readability. I might just end up replacing the names with the underscore ones, and keeping the variant that's better documented. But for now I just put them next to each other, so it's at least more obvious that we have this duplication (they used to be half a module apart, so I guess the duplication was missed until now). Also, diff coverage is complaining because I moved the unused (and untested) variants of these functions... |
Docs build fails (also after retry) with another ContentTooShortError, which I think is unrelated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions being unused and untested is reason enough to remove them, because now they are a liability, not an asset.
About the airmass function names, both names are bad. Maybe I should add the functions zenith_dist_from_airmass()
and airmass_from_zenith_dist()
to this file 😜!
Remove utils functions that were not used anywhere in the codebase (I also checked our other main packages).