Backport PR #3358 on branch v4.0.x (Fix aperture weight mask with loaded mask cube) #3360
Conversation
rosteen
added
bug
rosteen
requested review from
javerbukh,
pllim,
kecnry,
bmorris3,
haticekaratay,
cshanahan1 and
gibsongreen
as code owners
… mask cube Fix aperture weight mask with loaded mask cube (spacetelescope#3358) * Track loaded mask cube in cubeviz * Don't save DQ array as loaded_mask for JWST * Add extraction test on file with mask * Codestyle * Add consideration of loaded mask to aperture weight mask * Remove duplicate test, codestyle * Changelog * Update jdaviz/configs/cubeviz/plugins/tests/test_parsers.py Co-authored-by: P. L. Lim <[email protected]> * Cast this as bool here * Fix tests, address review comments * adding regression tests to manga mask * Update jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py Co-authored-by: Brett M. Morris <[email protected]> --------- Co-authored-by: P. L. Lim <[email protected]> Co-authored-by: Brett M. Morris <[email protected]> Co-authored-by: Brett M. Morris <[email protected]> (cherry picked from commit 92390e2)
rosteen
force-pushed
the
auto-backport-of-pr-3358-on-v4.0.x
branch
from
9a31b38 to
1cf6d9e
Compare
|
Argh, the new MaNGA test is still slightly off from the result on main, must be getting affected by something we didn't backport. |
|
I think it's because of the update to Cubeviz parsing that then required a minor fix for MaNGA...the difference is small enough (and we have an independent test that's showing this behavior is working) that I'm ok bumping the RTOL here at this point. |
Do you remember which PR? |
|
I think it must be #3307 |
|
(which fixed MaNGA loading after #3221) |
|
Tsk tsk... always Past Me that shows up lol... I agree that it is ok to bump rtol now. |
| se.extract() | ||
| extracted_max = cubeviz_helper.get_data("Spectrum (mean)").max() | ||
| assert_allclose(extracted_max.value, 2.836957E-18, rtol=1E-5) |
There was a problem hiding this comment.
given that the tolerance was larger before I suggested a smaller one, I don't feel bad bumping it up.
There was a problem hiding this comment.
Well, it was larger due to exactly this, and I had hopes that it wouldn't be necessary now.
There was a problem hiding this comment.
But I'll just bump it back up on 4.0.1 since there are no objections.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v4.0.x #3360 +/- ##
==========================================
- Coverage 88.62% 88.62% -0.01%
==========================================
Files 125 125
Lines 18819 18822 +3
==========================================
+ Hits 16679 16681 +2
- Misses 2140 2141 +1 ☔ View full report in Codecov by Sentry. |
Manual backport of #3358.