-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix bug in catalog search functionality and make comparison notebook compatible with CatalogData
#242
Conversation
This fixes feder-observatory#241 by allowing the location for a catalog search to be given as a WCS. Tests for setting the location were also added, as was a test for the case when a FITS header has no WCS.
This should have been done when the new CatalogData was implemented in
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #242 +/- ##
==========================================
+ Coverage 54.51% 55.22% +0.70%
==========================================
Files 23 23
Lines 2911 2959 +48
==========================================
+ Hits 1587 1634 +47
- Misses 1324 1325 +1 ☔ View full report in Codecov by Sentry. |
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.
See the comments, the logic was sound but there were several documentation issues that I could see.
Addresses a review comment pointing out that we already use "location" for the observatory.
I think I've addressed all of your comments, thanks for the review! |
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.
Almost there, missing a clarification of use of r-band filter in mag_scale
in comparison_utils.py
.
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.
Looks good!
This PR started as a way to get the comparison notebook working again. Along the way I found a couple of issues in
CatalogData.from_vizier
that needed fixing.Fixes #241