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.

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

Improve get_subsets performance for single subset case #3363

Merged
merged 7 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 7 additions & 4 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
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 data-menu. [#3220, #3254, #3263, #3264, #3271, #3272, #3274, #3289, #3310]

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

Cubeviz
^^^^^^^

Imviz
^^^^^

* Orientation plugin API now exposes create_north_up_east_left and create_north_up_east_right methods. [#3308]
- Orientation plugin API now exposes create_north_up_east_left and create_north_up_east_right methods. [#3308]

* Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]
- Add Roman WFI and CGI footprints to the Footprints plugin. [#3322]

- Catalog Search plugin now exposes a maximum sources limit for all catalogs and resolves an edge case
when loading a catalog from a file that only contains one source. [#3337]
Expand Down Expand Up @@ -86,9 +86,12 @@ Specviz2d

Other Changes and Additions
---------------------------

- Added a short description of each plugin in the side menu, visible before the plugin is opened. Removes redundant descriptions above link
out to documentation when plugin is opened. Enable search on plugin description in addition to title. [#3268]

- Improved performance of ``app.get_subsets`` for the single-subset case. [#3363]

4.0.1 (2024-12-16)
==================

Expand Down
17 changes: 11 additions & 6 deletions jdaviz/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,16 @@

all_subsets = {}

# If we only want one subset, no need to loop through them all
if subset_name not in (None, ""):
if isinstance(subset_name, str):
subsets = [subset for subset in subsets if subset.label == subset_name]
if subsets == []:
all_labels = sorted(sg.label for sg in dc.subset_groups)
raise ValueError(f"{subset_name} not in {all_labels}")
else:
raise ValueError("subset_name must be a string.")

Check warning on line 966 in jdaviz/app.py

View check run for this annotation

Codecov / codecov/patch

jdaviz/app.py#L966

Added line #L966 was not covered by tests

for subset in subsets:

label = subset.label
Expand Down Expand Up @@ -1020,13 +1030,8 @@
else:
all_subsets[label] = subset_region

# can this be done at the top to avoid traversing all subsets if only
# one is requested?
all_subset_names = [subset.label for subset in dc.subset_groups]
if subset_name and subset_name in all_subset_names:
if subset_name:
pllim marked this conversation as resolved.
Show resolved Hide resolved
return all_subsets[subset_name]
elif subset_name:
raise ValueError(f"{subset_name} not in {all_subset_names}")
else:
return all_subsets

Expand Down
Loading