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

data-menu: handle associated data layer logic #3370

Merged
merged 4 commits into from
Dec 20, 2024
Merged

data-menu: handle associated data layer logic #3370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9ea4b85
logic to connect visibilities of child/parent layers from data-menu
kecnry Dec 20, 2024
afa8c03
add to existing changelog entry
kecnry Dec 20, 2024
c2f1ba6
test coverage
kecnry Dec 20, 2024
f34f062
load image data and DQ together
kecnry Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
New Features
------------

- New design for viewer legend and data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289, #3310]
- New design for viewer legend and future data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289, #3310, #3370]

- Improve performance while importing multiple regions. [#3321]

Expand Down
9 changes: 9 additions & 0 deletions jdaviz/configs/default/plugins/data_menu/data_menu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,15 @@ def set_layer_visibility(self, layer_label, visible=True):
layer.visible = visible
elif hasattr(layer.layer, 'data') and layer.layer.data.label == layer_label:
layer.visible = layer.layer.label in self.visible_layers
if not visible and self.app._get_assoc_data_parent(layer.layer.label) == layer_label:
# then this is a child-layer of a parent-layer that is being hidden
# so also hide the child-layer
layer.visible = False

if visible and (parent_label := self.app._get_assoc_data_parent(layer_label)):
Copy link
Contributor

Choose a reason for hiding this comment

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

I am the walrus.

# ensure the parent layer is also visible
self.set_layer_visibility(parent_label, visible=True)
Comment on lines +366 to +368
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that you can make a DQ layer visible for a data entry that is currently invisible, and doing so also toggles the parent layer visibility? I think you can't do that in the current data menu, but I don't oppose to choosing that behavior going forward.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The current data-menu disables the ability to set a child-of-a-non-visible-parent to visible. That's also an option here, but since we don't (yet) have the design to visually tie them and don't have a clear way to denote that in the UI, I figured this might be a reasonable alternative. What would you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's good like this.


return self.visible_layers

def toggle_layer_visibility(self, layer_label):
Expand Down
28 changes: 28 additions & 0 deletions jdaviz/configs/default/tests/test_data_menu.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from astropy.io import fits
import pytest
import numpy as np
from specutils import SpectralRegion
Expand Down Expand Up @@ -146,6 +147,33 @@ def test_data_menu_remove_subset(specviz_helper, spectrum1d):
# assert dm.layer.choices == ['test', 'test2', 'Subset 2']


def test_data_menu_dq_layers(imviz_helper):
hdu_data = fits.ImageHDU(np.zeros(shape=(2, 2)))
hdu_data.name = 'SCI'
hdu_dq = fits.ImageHDU(np.zeros(shape=(2, 2), dtype=np.int32))
hdu_dq.name = 'DQ'
data = fits.HDUList([fits.PrimaryHDU(), hdu_data, hdu_dq])

imviz_helper.load_data(data, data_label="image", ext=('SCI', 'DQ'), show_in_viewer=True)

dm = imviz_helper.viewers['imviz-0']._obj.data_menu
assert dm.layer.choices == ['image[DQ,1]', 'image[SCI,1]']
assert len(dm._obj.visible_layers) == 2

# turning off image (parent) data-layer should also turn off DQ
dm.set_layer_visibility('image[SCI,1]', False)
assert len(dm._obj.visible_layers) == 0

# turning on image (parent) should leave DQ off
dm.set_layer_visibility('image[SCI,1]', True)
assert len(dm._obj.visible_layers) == 1

# turning on DQ (child, when parent off) should show parent
dm.set_layer_visibility('image[SCI,1]', False)
dm.set_layer_visibility('image[DQ,1]', True)
assert len(dm._obj.visible_layers) == 2


@pytest.mark.skip(reason="known issue")
def test_data_menu_subset_appearance(specviz_helper, spectrum1d):
# NOTE: this test is similar to above - the subset is appearing in time IF there
Expand Down
Loading