Add get_zeropoint and more flexible metadata querying to SVO FPS#3545
Add get_zeropoint and more flexible metadata querying to SVO FPS#3545keflavich wants to merge 7 commits intoastropy:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3545 +/- ##
==========================================
+ Coverage 72.66% 72.72% +0.06%
==========================================
Files 219 219
Lines 20480 20489 +9
==========================================
+ Hits 14882 14901 +19
+ Misses 5598 5588 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bsipocz
left a comment
There was a problem hiding this comment.
The changes look good, and tests are happy.
However, I'm less happy about introducing back the **kwarg approach. Could we maybe expand it out and list all the options explicitly? If nothing else, it would make the docstring more useful.
| ) | ||
|
|
||
| def get_filter_metadata(self, filter_id, *, cache=True, timeout=None): | ||
| def get_filter_metadata(self, filter_id, *, cache=True, timeout=None, **kwargs): |
There was a problem hiding this comment.
can we be more specific about the acceptable kwargs? Or is the list very long?
(I try to get rid of these wildcarded kwargs as they swallow a lot of problems as users can pass on practically anything even if it has no effect; besides it means that our documentation is practically useless.)
There is metadata on SVO FPS that is inaccessible but should be.
#3528 showed the right mechanism to retrieve metadata, but it didn't allow querying on PhotCalID - while
data_from_svoallowed specification of this parameter, the mechanism we are using to parse the votable drops the relevant data.This PR adds a new
get_zeropointmechanism to very explicitly and cleanly retrieve zeropoints and also generalizesget_filter_metadata.Note that I have made some serious mistakes in recent publications because I failed to realize the default zeropoint was vega, so it's high importance for me to expose the
get_zeropointmethod with an explicitmag_syskeyword to users.