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

improve laggy subset finalization #3175

Merged
merged 4 commits into from
Sep 5, 2024
Merged

improve laggy subset finalization #3175

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
a01b5a5
only call subset_selected_changed_callback for matching subset updates
kecnry Aug 29, 2024
0e9c1ca
optimize handling subset_state changes
kecnry Aug 29, 2024
45a3129
prevent unnecessary state-update
kecnry Aug 29, 2024
7b6b95d
avoid startswith since can be expensive on some machines
kecnry Aug 29, 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
14 changes: 8 additions & 6 deletions jdaviz/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ def _on_layers_changed(self, msg):
self.state.layer_icons = {
**self.state.layer_icons,
layer_name: alpha_index(len([ln for ln, ic in self.state.layer_icons.items()
if not ic.startswith('mdi-') and
if not ic[:4] == 'mdi-' and
self._get_assoc_data_parent(ln) is None]))
}

Expand All @@ -614,12 +614,14 @@ def _on_layers_changed(self, msg):
if children_layers is not None:
parent_icon = self.state.layer_icons[layer_name]
for i, child_layer in enumerate(children_layers, start=1):
child_layer_icons[child_layer] = f'{parent_icon}{i}'
if child_layer not in self.state.layer_icons:
child_layer_icons[child_layer] = f'{parent_icon}{i}'

self.state.layer_icons = {
**self.state.layer_icons,
**child_layer_icons
}
if child_layer_icons:
self.state.layer_icons = {
**self.state.layer_icons,
**child_layer_icons
}
Comment on lines -617 to +624
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: @bmorris3 - does this seem reasonable? I'm trying to avoid an update to the state again if no new child layers are added.


def _change_reference_data(self, new_refdata_label, viewer_id=None):
"""
Expand Down
3 changes: 2 additions & 1 deletion jdaviz/configs/cubeviz/plugins/spectral_extraction/spectral_extraction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ def _aperture_items_changed(self, msg):
return
if not hasattr(self, 'aperture'):
return
orig_labels = [item['label'] for item in msg['old']]
for item in msg['new']:
if item not in msg['old']:
if item['label'] not in orig_labels:
if item.get('type') != 'spatial':
continue
subset_lbl = item.get('label')
Expand Down
12 changes: 6 additions & 6 deletions jdaviz/core/template_mixin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1936,14 +1936,14 @@ def _update_subset(self, subset, attribute=None):
or (subset.label == self.selected))):
# updated the currently selected subset, clear all cache
self._clear_cache()
update_has_subregions = True
selected_has_changed = True
else:
update_has_subregions = False
selected_has_changed = False

if subset.label not in self.labels:
# NOTE: this logic will need to be revisited if generic renaming of subsets is added
# see https://github.com/spacetelescope/jdaviz/pull/1175#discussion_r829372470
if subset.label.startswith('Subset') and self._is_valid_item(subset):
if subset.label[:6] == 'Subset' and self._is_valid_item(subset):
# NOTE: += will not trigger traitlet update
self.items = self.items + [self._subset_to_dict(subset)] # noqa
else:
Expand All @@ -1958,11 +1958,11 @@ def _update_subset(self, subset, attribute=None):
else self._subset_to_dict(subset)
for s in self.items]

if update_has_subregions:
if selected_has_changed:
self._update_has_subregions()

if self._subset_selected_changed_callback is not None:
self._subset_selected_changed_callback()
if self._subset_selected_changed_callback is not None:
self._subset_selected_changed_callback()

def _update_has_subregions(self):
if "selected_has_subregions" in self._plugin_traitlets.keys():
Expand Down