-
Notifications
You must be signed in to change notification settings - Fork 13
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 attribute handling for Jdaviz/Cubeviz parser #56
Conversation
Codecov Report
@@ Coverage Diff @@
## main #56 +/- ##
=======================================
Coverage 96.03% 96.03%
=======================================
Files 15 15
Lines 1136 1136
=======================================
Hits 1091 1091
Misses 45 45
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
The code is too sensitive. There are Glue subtleties that I am not familiar with, so any small refactoring breaks stuff. I have undone the refactoring and just left in a minimal fix that would hopefully fix Cami's use cases. |
py36 failure is unrelated. I don't think you can get Python 3.6 on Azure anymore but fixing that is out of scope. |
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 to me now - the data that was failing to load loads, and all the Jdaviz tests pass locally on my machine.
@astrofrog or @eteq , any chance you can merge and do a release with this fix? |
Thanks for the review, @rosteen ! |
Description
This PR fixes regression in Cubeviz parser behavior introduced in spacetelescope/jdaviz#547 , particularly getting rid of the assumption that "uncertainty" is always available when "flux" is available, which is not always true.
Also refactor the internal logic a little.Fixes spacetelescope/jdaviz#967