jdaviz
jdaviz copied to clipboard
Hide spectral subsets in cubeviz
Description
https://github.com/spacetelescope/jdaviz/assets/42986583/201e5f33-0592-40ff-8000-b8585bceb946
Behavior in the video displays that the initial overlay when a slice is viewed is still visible in flux/uncert viewers. After a state change, overlays become hidden.
When _expected_subset_layer_default() is called in on_layers_update(), the subset is either created and but the data is inaccessible, or the glue message is stating that the subset is in the process of being create, in either case, resulting in type None for data contained within the subset.
Looking into the message created from _on_subset_create(), yields the same subset data of type None. Tracing through the remaining default viewer class methods yields the same conclusion.
Note:
- By inferring the subset type is possible for the uncert-viewer, this can be done by accessing the data label, and using the string ending to turn off this overlay at the beginning, while still enabling spatial subsets to be shown on the viewer.
- The same assumption may be possible with the flux-viewer, although, the SCI data is used for both flux & spectrum-viewers, and so the share the same label. If there was a separate differentiation, then this may also be possible.
Fixes #<Issue Number>
Change log entry
- [x] Is a change log needed? If yes, is it added to
CHANGES.rst? If you want to avoid merge conflicts, list the proposed change log here for review and add toCHANGES.rstbefore merge. If no, maintainer should add ano-changelog-entry-neededlabel.
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.
- [ ] Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the
triviallabel. - [ ] Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
- [ ] Do the proposed changes follow the STScI Style Guides?
- [ ] Are tests added/updated as required? If so, do they follow the STScI Style Guides?
- [ ] Are docs added/updated as required? If so, do they follow the STScI Style Guides?
- [ ] Did the CI pass? If not, are the failures related?
- [ ] Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
- [ ] After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?
By inferring the subset type is possible for the uncert-viewer, this can be done by accessing the data label, and using the string ending to turn off this overlay at the beginning, while still enabling spatial subsets to be shown on the viewer.
I don't think this is reliable enough - we do allow loading the flux cube into the uncertainty viewer (and vice versa).
If its not possible to access the information we need at available callbacks/methods, then I think we should make an upstream request for a message to be sent after a subset is created and fully available, and then make use of that here.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Comparison is base (
09d7da6) 91.52% compared to head (cf3d3f5) 91.52%. Report is 6 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #2636 +/- ##
=======================================
Coverage 91.52% 91.52%
=======================================
Files 161 161
Lines 19969 19982 +13
=======================================
+ Hits 18277 18289 +12
- Misses 1692 1693 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
By inferring the subset type is possible for the uncert-viewer, this can be done by accessing the data label, and using the string ending to turn off this overlay at the beginning, while still enabling spatial subsets to be shown on the viewer.
I don't think this is reliable enough - we do allow loading the flux cube into the uncertainty viewer (and vice versa).
If its not possible to access the information we need at available callbacks/methods, then I think we should make an upstream request for a message to be sent after a subset is created and fully available, and then make use of that here.
I was thinking that it was a pretty hard-coded way of doing too so I didn't include it in the file changes nor demo it, and now that you mention it, the vis versa, was one thing that I was doing to test the state changes.
All in all I think the upstream change like we talked about in code review is the best past forward.
@gibsongreen do you know if your subsets are created directly via
https://github.com/glue-viz/glue/blob/1981b686b3ba9df86a45e8cf00d190cf09e0fbe6/glue/core/data.py#L214
or using new_subset_group as indicated in the docstring there?
@gibsongreen do you know if your subsets are created directly via https://github.com/glue-viz/glue/blob/1981b686b3ba9df86a45e8cf00d190cf09e0fbe6/glue/core/data.py#L214 or using
new_subset_groupas indicated in the docstring there?
Hey @dhomeier , it does not appear that we use add_susbset()within jdaviz, we do seem to use new_subset_group