-
Notifications
You must be signed in to change notification settings - Fork 74
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
BUG: Metadata viewer to ignore invalid FITS header #1342
Conversation
instead of crashing.
Codecov Report
@@ Coverage Diff @@
## main #1342 +/- ##
==========================================
- Coverage 84.89% 84.64% -0.26%
==========================================
Files 91 91
Lines 7877 7878 +1
==========================================
- Hits 6687 6668 -19
- Misses 1190 1210 +20
Continue to review full report at Codecov.
|
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.
Are there other exceptions this might hit than the VerifyError
I saw? Or do you think it's just not worth importing VerifyError
from...wherever that lives? EDIT: this is in reference to the except Exception
, didn't comment inline for some reason.
@rosteen , I could specifically check for |
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.
I think I'm ok with it this way.
Thanks for the review! |
Description
This pull request is to avoid Metadata Viewer from crashing when user loads a file with invalid FITS header. Also see spacetelescope/dat_pyinthesky#177 . This is split from #1333 . As noted in the other PR, I have a very hard time creating invalid FITS header by hand and I do not want to download a whole cube to test this one thing, so I did not add any test.
This fixes an unreleased feature (#1325), so it does not need a change log.
Would be nice to merge this fast so I can build #1333 on top of this.
Checklist for package maintainer(s)
This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.
trivial
label.CHANGES.rst
?