Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Backport PR #3319 on branch v4.0.x (Respect loaded mask cube properly in spectral extraction.) #3348

Open
wants to merge 6 commits into
base: v4.0.x
Choose a base branch
from
Open

Backport PR #3319 on branch v4.0.x (Respect loaded mask cube properly in spectral extraction.) #3348

wants to merge 6 commits into from
+66 −15

Conversation

rosteen
Copy link
Collaborator

@rosteen rosteen commented Dec 11, 2024

Backporting #3319

mariobuikhuizen and others added 4 commits November 18, 2024 12:18
@mariobuikhuizen @rosteen
fix: golden layout not rendering when created outside viewport (space…
cb51f88 
…telescope#3299)

* fix: golden layout not rendering when created outside viewport

Since Lab 4.2 cells outside the viewport get the style
"display: none" which causes the content to not have height.
This causes an error in size calculations of golden layout
from which it doesn't recover.

* Add changelog

---------

Co-authored-by: Ricky O'Steen <rosteen@stsci.edu>
(cherry picked from commit 7e5ddfa)
@rosteen
Fix changelog header
415845d
@rosteen
fix conflict
c4efc14
@rosteen
Respect loaded mask cube properly in spectral extraction. (spaceteles…
8a966fe 
…cope#3319)

* Track loaded mask cube in cubeviz

* First pass at handling mask cube in extraction

* Fix type

* Add changelog

* Handle mask cubes that have NaNs instead of 0s

* Act on copy of this mask array

* Don't save DQ array as loaded_mask for JWST

* Add snackbar warning and note in docs

* codestyle

* Add extraction test on file with mask

* Codestyle

(cherry picked from commit bf8cb26)
@rosteen rosteen added bug Something isn't working cubeviz specviz plugin Label for plugins common to multiple configurations labels Dec 11, 2024
@rosteen rosteen added this to the 4.0.1 milestone Dec 11, 2024
@github-actions github-actions bot added the documentation Explanation of code and concepts label Dec 11, 2024
pllim
pllim approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@pllim
Copy link
Contributor

pllim commented Dec 11, 2024

Is there another PR need backporting first?

E DeprecationWarning: pkg_resources is deprecated as an API.

@pllim
Copy link
Contributor

pllim commented Dec 11, 2024

Maybe you can cherry pick that into this PR too.

@rosteen
Copy link
Collaborator Author

rosteen commented Dec 11, 2024

Is there another PR need backporting first?

E DeprecationWarning: pkg_resources is deprecated as an API.

Ahh, #3327 wasn't marked with the backport label. I'll cherry pick it here (or actually, will adding the label now still pick it up automatically?).

@bmorris3 @rosteen
Removing filter warning for mpl-scatter-density (spacetelescope#3327)
3d1a177 
* removing filter warning for mpl-scatter-density

* replace deprecated pkg_resources call in linelists

(cherry picked from commit 3f1abf7)
@rosteen rosteen mentioned this pull request Dec 11, 2024
Merged
9 tasks
@rosteen
Copy link
Collaborator Author

rosteen commented Dec 11, 2024

I cherry picked the commit from @bmorris3's PR fixing the deprecation warning to here.

@rosteen rosteen added the no-changelog-entry-needed changelog bot directive label Dec 11, 2024
@pllim
Copy link
Contributor

pllim commented Dec 11, 2024

Hmm is there something else?

jdaviz/configs/cubeviz/plugins/tests/test_parsers.py::test_manga_cube - AssertionError: 
Not equal to tolerance rtol=1e-07, atol=0

Mismatched elements: 1 / 1 (100%)
Max absolute difference among violations: 7.23287482e-24
Max relative difference among violations: 2.54951866e-06
 ACTUAL: array(2.836964e-18)
 DESIRED: array(2.836957e-18)

@rosteen
Copy link
Collaborator Author

rosteen commented Dec 11, 2024

Hmm is there something else?

jdaviz/configs/cubeviz/plugins/tests/test_parsers.py::test_manga_cube - AssertionError: 
Not equal to tolerance rtol=1e-07, atol=0

Mismatched elements: 1 / 1 (100%)
Max absolute difference among violations: 7.23287482e-24
Max relative difference among violations: 2.54951866e-06
 ACTUAL: array(2.836964e-18)
 DESIRED: array(2.836957e-18)

It looks like this is just a slightly too small RTOL on a very small number, the test has previously passed and this was just slightly outside the range. I think it's a precision thing.

@codecov Codecov
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.

Project coverage is 88.64%. Comparing base (d975cfa) to head (8ee5da2).
Report is 2 commits behind head on v4.0.x.

Files with missing lines Patch % Lines
jdaviz/configs/cubeviz/plugins/parsers.py 37.50% 5 Missing ⚠️
...plugins/spectral_extraction/spectral_extraction.py 92.85% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           v4.0.x    #3348   +/-   ##
=======================================
  Coverage   88.64%   88.64%           
=======================================
  Files         125      125           
  Lines       18795    18813   +18     
=======================================
+ Hits        16661    16677   +16     
- Misses       2134     2136    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pllim
Copy link
Contributor

pllim commented Dec 11, 2024

Given the discussion on #3350 , should we revert the rtol change here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pllim pllim pllim approved these changes

@javerbukh javerbukh Awaiting requested review from javerbukh javerbukh is a code owner

@kecnry kecnry Awaiting requested review from kecnry kecnry is a code owner

@bmorris3 bmorris3 Awaiting requested review from bmorris3 bmorris3 is a code owner

@haticekaratay haticekaratay Awaiting requested review from haticekaratay haticekaratay is a code owner

@cshanahan1 cshanahan1 Awaiting requested review from cshanahan1 cshanahan1 is a code owner

@gibsongreen gibsongreen Awaiting requested review from gibsongreen gibsongreen is a code owner

Assignees
No one assigned
Labels
bug Something isn't working cubeviz documentation Explanation of code and concepts no-changelog-entry-needed changelog bot directive plugin Label for plugins common to multiple configurations specviz
Projects
None yet
Milestone
4.0.1
Development

Successfully merging this pull request may close these issues.
4 participants
@rosteen @pllim @mariobuikhuizen @bmorris3