Skip to content

Commit 5e78640

Browse files
Add WFSS L3 compatibility and search to plugin select dropdowns (#3729)
* Temporary test for new WFSS L3 files * Starting work on WFSS L3 loader * Added warning message for masked spectra * Prepping for vue things * Revert to using dropdown for simplicity * This may change to the table in the future once I've got a handle on using the dropdown. * This is (closely) following the prescription in the image importer * Added SelectSpectraComponent * Specifically for the SpectrumList importer. * WIP testing with known spectrum list file * WIP working on preloading spectra for user choice * WIP working on getting spectra to load correctly with mask * WIP added mask checking/application logic * WIP fixed bugs, fixed some logic * also working on tests to confirm this operates as expected * WIP fixing up and adding to SelectSpectraComponent * WIP prototype working version * Uses exposure number + source ID to create the unique label/reference * Will likely move checking for issues with creating a spectrum in the specutils parser rather than applying masks everywhere * WIP spectrum_list works as expected * Need to tidy things up and fix some display things. * Likely need to mess with labels. * Modified checks and generalized how the spectra show in the loader (index based if WFSS source_id can't be found). * Also fixed up some things in the tests but they have to be formalized of course. * WIP include search bar * It's a bit rudimentary but it'll work for now. May attempt to clean it up later. * Added v-if to check for search bool * WIP added search, need to work on data labels * Also remove list concatenation for now to speed up testing * WIP still working on tests * Added TODO * Made search a bit more graceful * Removed default option * Still trying to make the search function more seamless * Added padding below dropdown * Updated data label in viewer * Fix TODOs * Vectorize SpectrumListConcatenated * Some options to handle SpectrumListConcatenated * Cleaned up some code * Added disabling for when search is not necessary * Mostly for SpectrumListConcatenated * Reduced padding a bit * Refactor logic for consistent labeling * Remove most logic from output and put it into init where the action is happening anyway * Add in suffix to options dict to keep everything together * Modify SpectrumListConcatenated to avoid error due to no spectra selection since we're combining them all * Use selected_obj_dict instead of selected_obj * This allows us to keep everything together easier for labeling and whatnot * Updated changelog * Added TODO * Remove search function * Too awkward as is, will be implemented in plugin_select later * Added warning for duplicated data labels * Only show warning once... to avoid inundating the user on back-to-back imports * Initial commit of tests * Made some fixtures and an initial test for is_valid * Debugging getting some tests in * Working on proper setup before moving forward * Removed some unused code and slightly modified masking * Fixed up tests to use custom test class * Finally figured out the commands to instantiate the class directly... * This allows some convenience when changing input and also avoids dealing with the rest of the loader structure. * Commented out tests were generated by copilot and will be reviewed * Reverted for now * Fixed style issues * Fixed style issues * Fixed style issues * Fixed style issues * Remove old commented out code * Adjusted explanation for spectral_mask * Added a TODO and a comment about masks * Updated tests to work correctly with masking * Removed fully masked entries, Spectum objects can't be initialized fully masked, but they can have the masks updated afterward. * Added a TODO and a comment about masks * Remove SelectSpectraComponent to use SelectFileExtensionComponent * They're functionally the same except I also added the ability to retrieve the original dict (and included a suffix key for data label purposes). * Changes to account for viewer issues * Included parent logic which helps keep spectra from the same file together however there is still the issue that multiple WFSS files load into the same viewer. * When parenting+add to viewer happened at the same time in the loop, there would be an error when importing non WFSS + WFSS which stated that the dataset could not be found in `self.dataset.choices`. I could not find the source of this issue but managed to mitigate that by adding to the viewer after all the data had been added to the collection. I don't think this should happen, so I left a TODO in. * Building out more tests * Updated TODOs after discussion * Added more tests, still WIP * Fixed some bugs found through testing * Adjusted naming for non-WFSS files to clarify file indices vs internal indices. * Add split check for index based naming for existing data labels. * Moved previous_data_label_messages to init to avoid overwrite on new call. * Move previous_data_label_messages inside if to avoid duplicates. * Use `default_viewer_reference` instead of hardcoded viewer for adding to viewer. * Added a bunch of tests * Some TODOs left over due to known bugs or odd behavior. * Changed spectra meta to avoid duplicates and the headaches that that causes when forcing those to be ingested. * Use custom fake class to override default_data_label_resolver to avoid labeling duplication and issues when checking viewer (although I still couldn't get a second viewer to pop out, at least naming is easier). * Added mock patch for snackbar catching. * re-arranged the code a bit. * Added some basic concatenated tests... I may revisit these before merging the PR. * Small adjustments to code for consistency/bugs * Added check and default behavior for no spectra selected... for compatability with config.load which doesn't specify a selection but needs something to be selected in order to avoid an error in get_selected_obj. * Added observer to disable import button when nothing is selected (and to avoid the case where the user accidentally imports everything due to the selection being none -> all). * Added a warning for the user if this does happen during the load process to save them the stress of wondering what's happening when it's attempting to load all the data. * Added TODO in regards to the warning * Added some tests, parametrized others, added some checks * Notably checking initialization * Also added tests for newly added functions in spectrum_list * Adjust for changes in spectrum list importer * Made several helper methods private * Adjusted test for new functionality and added new WFSS test * Fake Importers are added to the registry for the new spectrum_list tests so I had to exclude those in some of the old tests. I have added a fixture to conftest to help with this in case it comes up in the future. * The WFSS file test is basic but should be OK because the hardcore testing happens in test_spectrum_list.py * Style fixes * Style fixes * Adapt existing fixtures for spectrum list importer tests and vice versa * Style fixes * Added a docstring-ish comment to explain new fixture * Fix tests to avoid warnings on CI * Style fixes * Found a couple more warning spots to fix * Style fix * Made observing more dynamic * Since we determine which loader to use by checking them all, I was accidentally disabling import for the other possible loaders when attempting to use the spectrum_list loader without anything selected. This has now been fixed with two separate observers, one for format and one for spectra_selected. * Updated tests * Small adjustments * Fixed and added tests * Added tests for new observe functionality * Fixed some small things in other tests * Get coverage of snackbar message for fully masked spectra * Style fixes * Style fixe * Change label name for quality of life * This does not affect the label that is output although it may just a hair more confusing for being generic vs the descriptive data label in the viewer. * Implemented search in plugin_select.vue * Search now shows up in dropdown and actively filters (similar to as in app.vue). * Added some padding below the dropdown box. * Added exposures dropdown * Used for WFSS fits files and potentially others in the future * Added traitlets in spectrum_list.py * Added methods and logic for filtering the source id dropdown by exposures. * Update API hints * Added tests for exposures stuff * Removed no-longer applicable test * Remove TODO * Updated variable names to follow convention * Updated CHANGES.rst * Updated CHANGES.rst * Update CHANGES.rst * Update CHANGES.rst * Removed references to parenting... in preparation for a PR to address the viewer issue. * Add staticmethod decorators * Renamed variable to avoid shadowing * Add TODO for skipped test * Add `load_all` kwarg option for spectrum lists * Update tests with load_all functionality * Add load_selected and load_all functionality * Condense tests and TODO * Cover new load_selected attribute * Update error message for none selected * Remove load_all, allow wildcard filtering in load_selected * Add dropdown for selection to SpectrumListConcatenated * Update CHANGES.rst Co-authored-by: Kyle Conroy <[email protected]> * Revert to using `disable_dropdown` except for concatenated 2d spectra * add coverage * Add unit test for `_load_selected_helper` * Fixed a bad assert in test * Remove fully_masked_spectra list * It seems the GUI can handle it after all! * Style fixes * Style fixes * Update CHANGES.rst * Moved source_id filtering to backend * Use observer on exposure selection change to propagate this * Added and modified tests to accommodate this * Moved source_id filtering to backend * Use observer on exposure selection change to propagate this * Added and modified tests to accommodate this * Add `suffix` to the list of keys in `SelectFileExtensionComponent` items and modified `spectrum_list` therein. Removed `get_selected_obj_dict`. * Modified load_selected logic... to use existing '*' wildcard attribute setting in `user_api` to select all. * Remove `load_selected` and related functionality * Adjust tests to use `user_api.spectra` instead * Small adjustments and update tests * Expose `exposures`. Don't set `exposures.selected` to an empty list, just check for `None`. * Move check for 2D spectra into `SpectrumListConcatenatedImporter` init * Use `spectra.select_all()` instead of '*'. * Fix API hints * Removed custom data_label check logic * Style fixes * Adjustment to label * Change `spectra` to `sources` * And other small bugs * Updated vue file with spectra->sources * Restored exposures.selected to initialize with empty list * The dropdown didn't show otherwise and this is easier to remove in the future when the multiselect bug is fixed than other logic to guarantee it being there. * Revert tests for exposures.selected initialization * Added fully_masked test back and more complicated tests for ...selection_change * Tentative commit for more extensive logic for format/selection change * Minor fixes and comments * Style fixes * 2d spectrum as 1d should always convert to flux units * clarify flux vs spectral flux density * test coverage * implement toggle to control whether to convert to flux density * Fixes to SpectrumListConcatenated tests * Accidentally included code from Kyle's PR * codestyle * No need to check is_valid in init anymore * Updated test to remove old check * Style fix * Fixed tab issue from merge * Tab issue * Style fix * Style fix * Comment out SB conversion toggle for now * Removed exposure dropdown and references therein * Remove unused imports * Force the flux to be read in from the specified flux column... to avoid issues with different source types (point vs extended resulted in flux vs surface brightness which caused a big headache for everyone) * Removed references to unit conversion, (properly) force flux column * Restoring unit conversion code --------- Co-authored-by: Kyle Conroy <[email protected]>
1 parent 6bbb435 commit 5e78640

File tree

13 files changed

+1153
-126
lines changed

13 files changed

+1153
-126
lines changed

CHANGES.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ New Features
77
- The Model Fitting plugin now allows the user to select a fitter from a dropdown menu, with the default being the
88
``astropy.modeling.fitting.TRFLSQFitter``. [#3720]
99

10+
- Dropdown menus generated by ``plugin_select.vue`` now have the ability to be searched when
11+
enabled. [#3729]
12+
1013
Cubeviz
1114
^^^^^^^
1215

@@ -19,6 +22,8 @@ Mosviz
1922
Specviz
2023
^^^^^^^
2124

25+
- Added support for WFSS Level 3 data. [#3729]
26+
2227
Specviz2d
2328
^^^^^^^^^
2429

@@ -61,6 +66,8 @@ Specviz2d
6166
Other Changes and Additions
6267
---------------------------
6368

69+
- When importing a 2D spectrum file into a SpectrumList, surface brightness units are automatically converted to flux units. [#3729]
70+
6471
4.3.1 (unreleased)
6572
==================
6673

jdaviz/components/plugin_select.vue

Lines changed: 90 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<v-select
44
:menu-props="{ left: true }"
55
attach
6-
:items="items"
6+
:items="filtered_items"
77
v-model="selected"
88
@change="$emit('update:selected', $event)"
99
:label="api_hints_enabled && api_hint ? api_hint : label"
@@ -18,48 +18,56 @@
1818
item-value="label"
1919
persistent-hint
2020
style="width: 100%"
21+
:search-input="search_enabled ? search_query : undefined"
22+
@update:search-input="on_search_input"
2123
>
22-
<template v-slot:selection="{ item, index }">
24+
<template #prepend-item>
25+
<div v-if="search_enabled">
26+
<v-text-field
27+
v-model="search_query"
28+
prepend-inner-icon="mdi-magnify"
29+
label="Search"
30+
single-line
31+
hide-details
32+
autofocus
33+
style="margin: 8px;"
34+
/>
35+
<v-divider class="mt-2"></v-divider>
36+
</div>
37+
<div v-if="multiselect">
38+
<v-list-item
39+
ripple
40+
@mousedown.prevent
41+
@click="toggle_select_all"
42+
>
43+
<v-list-item-action>
44+
<v-icon>
45+
{{ selected.length === items.length ? 'mdi-close-box' : selected.length ? 'mdi-minus-box' : 'mdi-checkbox-blank-outline' }}
46+
</v-icon>
47+
</v-list-item-action>
48+
<v-list-item-content>
49+
<v-list-item-title>
50+
{{ selected.length < items.length ? 'Select All' : 'Clear All' }}
51+
</v-list-item-title>
52+
</v-list-item-content>
53+
</v-list-item>
54+
<v-divider class="mt-2"></v-divider>
55+
</div>
56+
</template>
57+
<template #selection="{ item, index }">
2358
<div class="single-line" style="width: 100%">
24-
<span v-if="api_hints_enabled" class="api-hint" :style="index > 0 ? 'display: none' : null">
25-
{{ multiselect ?
26-
selected
27-
:
28-
'\'' + selected + '\''
29-
}}
59+
<span v-if="api_hints_enabled && index === 0" class="api-hint">
60+
{{ multiselect ? selected : `'${selected}'` }}
3061
</span>
3162
<v-chip v-else-if="multiselect" style="width: calc(100% - 10px)">
32-
<span>
33-
{{ item }}
34-
</span>
63+
<span>{{ item }}</span>
3564
</v-chip>
36-
<span v-else>
37-
{{ item }}
38-
</span>
65+
<span v-else>{{ item }}</span>
3966
</div>
4067
</template>
41-
<template v-slot:prepend-item v-if="multiselect">
42-
<v-list-item
43-
ripple
44-
@mousedown.prevent
45-
@click="() => {if (selected.length < items.length) { $emit('update:selected', items.map((item) => {return item.label || item.text || item}))} else {$emit('update:selected', [])}}"
46-
>
47-
<v-list-item-action>
48-
<v-icon>
49-
{{ selected.length == items.length ? 'mdi-close-box' : selected.length ? 'mdi-minus-box' : 'mdi-checkbox-blank-outline' }}
50-
</v-icon>
51-
</v-list-item-action>
52-
<v-list-item-content>
53-
<v-list-item-title>
54-
{{ selected.length < items.length ? "Select All" : "Clear All" }}
55-
</v-list-item-title>
56-
</v-list-item-content>
57-
</v-list-item>
58-
<v-divider class="mt-2"></v-divider>
59-
</template>
60-
<template v-slot:item="{ item }">
68+
<template #item="{ item }">
6169
<span style="margin-top: 8px; margin-bottom: 0px">
62-
{{ item }}
70+
{{ typeof item === 'string' ? item : (item.label || item.text) }}
6371
</span>
6472
</template>
6573
</v-select>
@@ -70,7 +78,53 @@
7078
<script>
7179
module.exports = {
7280
props: ['items', 'selected', 'label', 'hint', 'rules', 'show_if_single_entry', 'multiselect',
73-
'api_hint', 'api_hints_enabled', 'dense', 'disabled']
81+
'api_hint', 'api_hints_enabled', 'dense', 'disabled', 'search'],
82+
data() {
83+
return {
84+
search_query: '',
85+
}
86+
},
87+
computed: {
88+
search_enabled() {
89+
return !!this.search;
90+
},
91+
filtered_items() {
92+
if (!this.search_enabled || !this.search_query) {
93+
return this.items;
94+
}
95+
const query = this.search_query.toLowerCase();
96+
// Get selected items (as objects or strings)
97+
const selected_set = new Set(
98+
Array.isArray(this.selected) ? this.selected.map(sel => (typeof sel === 'string' ? sel : sel.label || sel.text || sel)) : [this.selected]
99+
);
100+
// Filter items by search
101+
const filtered = this.items.filter(item => {
102+
const label = (typeof item === 'string') ? item : (item.label || item.text || '');
103+
return label.toLowerCase().includes(query);
104+
});
105+
// Add back any selected items not in filtered
106+
const all_labels = new Set(filtered.map(item => (typeof item === 'string' ? item : item.label || item.text || '')));
107+
this.items.forEach(item => {
108+
const label = (typeof item === 'string') ? item : (item.label || item.text || '');
109+
if (selected_set.has(label) && !all_labels.has(label)) {
110+
filtered.push(item);
111+
}
112+
});
113+
return filtered;
114+
},
115+
},
116+
methods: {
117+
on_search_input(value) {
118+
this.search_query = value;
119+
},
120+
toggle_select_all() {
121+
if (this.selected.length < this.items.length) {
122+
this.$emit('update:selected', this.items.map(item => item.label || item.text || item));
123+
} else {
124+
this.$emit('update:selected', []);
125+
}
126+
},
127+
},
74128
};
75129
</script>
76130

jdaviz/configs/specviz/helper.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def __init__(self, *args, **kwargs):
4646
@deprecated(since="4.3", alternative="load")
4747
def load_data(self, data, data_label=None, format=None, show_in_viewer=True,
4848
concat_by_file=False, cache=None, local_path=None, timeout=None,
49-
load_as_list=False):
49+
load_as_list=False, exposures=None, sources=None, load_all=False):
5050
"""
5151
Load data into Specviz.
5252
@@ -90,6 +90,10 @@ def load_data(self, data, data_label=None, format=None, show_in_viewer=True,
9090
load_kwargs['timeout'] = timeout
9191
if local_path is not None:
9292
load_kwargs['local_path'] = local_path
93+
if sources is not None:
94+
load_kwargs['sources'] = sources
95+
if exposures is not None:
96+
load_kwargs['exposures'] = exposures
9397

9498
if isinstance(data, (SpectrumList, SpectrumCollection)) and isinstance(data_label, list):
9599
if len(data_label) != len(data):

jdaviz/configs/specviz/plugins/unit_conversion/unit_conversion.py

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
create_equivalent_flux_units_list,
2121
check_if_unit_is_per_solid_angle,
2222
create_equivalent_angle_units_list,
23-
supported_sq_angle_units)
23+
flux_to_sb_unit)
2424

2525
__all__ = ['UnitConversion']
2626

@@ -43,16 +43,6 @@ def _valid_glue_display_unit(unit_str, viewer, axis='x'):
4343
return choices_str[ind]
4444

4545

46-
def _flux_to_sb_unit(flux_unit, angle_unit):
47-
if angle_unit not in supported_sq_angle_units(as_strings=True):
48-
sb_unit = flux_unit
49-
else:
50-
# str > unit > str to remove formatting inconsistencies with
51-
# parentheses/order of units/etc
52-
sb_unit = (u.Unit(flux_unit) / u.Unit(angle_unit)).to_string()
53-
return sb_unit
54-
55-
5646
@tray_registry('g-unit-conversion', label="Unit Conversion",
5747
category="core", sidebar="settings", subtab=1)
5848
class UnitConversion(PluginTemplateMixin):
@@ -395,17 +385,17 @@ def _on_unit_selected(self, msg):
395385
# which in turn will call _handle_attribute_display_unit,
396386
# _handle_spectral_y_unit (if spectral_y_type_selected == 'Surface Brightness'),
397387
# and send a second GlobalDisplayUnitChanged message for sb
398-
self.sb_unit_selected = _flux_to_sb_unit(self.flux_unit.selected,
399-
self.angle_unit.selected)
388+
self.sb_unit_selected = flux_to_sb_unit(self.flux_unit.selected,
389+
self.angle_unit.selected)
400390

401391
elif axis == 'angle':
402392
if len(self.flux_unit_selected):
403393
# NOTE: setting sb_unit_selected will call this method again with axis=='sb',
404394
# which in turn will call _handle_attribute_display_unit,
405395
# _handle_spectral_y_unit (if spectral_y_type_selected == 'Surface Brightness'),
406396
# and send a second GlobalDisplayUnitChanged message for sb
407-
self.sb_unit_selected = _flux_to_sb_unit(self.flux_unit.selected,
408-
self.angle_unit.selected)
397+
self.sb_unit_selected = flux_to_sb_unit(self.flux_unit.selected,
398+
self.angle_unit.selected)
409399

410400
elif axis == 'sb':
411401
# handle spectral y-unit first since that is a more apparent change to the user

jdaviz/configs/specviz/tests/test_helper.py

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import numpy as np
44
import pytest
5+
import re
56
from astropy import units as u
67
from astropy.io import fits
78
from astropy.tests.helper import assert_quantity_allclose
@@ -55,24 +56,30 @@ def test_load_hdulist(self):
5556
# HDUList should load as Spectrum
5657
assert isinstance(data, Spectrum)
5758

58-
def test_load_spectrum_list_no_labels(self):
59-
# now load three more spectra from a SpectrumList, without labels
60-
self.spec_app.load_data(self.spec_list)
61-
assert len(self.spec_app.app.data_collection) == 4
62-
for i in (1, 2, 3):
63-
assert "1D Spectrum" in self.spec_app.app.data_collection[i].label
64-
65-
def test_load_spectrum_list_with_labels(self):
66-
# NOTE: will be removed after load_data deprecation is removed
67-
# now load three more spectra from a SpectrumList, with labels:
68-
labels = ["List test 1", "List test 2", "List test 3"]
69-
self.spec_app.load_data(self.spec_list, data_label=labels)
59+
@pytest.mark.parametrize(
60+
'kwargs',
61+
({'data_label': [f"List test {i}" for i in (1, 2, 3)]},
62+
{'sources': [f"1D Spectrum at index: {i}" for i in (0, 1, 2)]},
63+
{'sources': '*'}))
64+
def test_load_spectrum_list_with_kwargs(self, kwargs):
65+
error_msg = "No sources selected."
66+
with pytest.raises(
67+
ValueError,
68+
match=re.escape(error_msg)):
69+
self.spec_app.load_data(self.spec_list)
70+
71+
# When loading via the ``data_label`` argument, the length of the
72+
# list must match the number of sources in the SpectrumList.
73+
self.spec_app.load_data(self.spec_list, **kwargs)
7074
assert len(self.spec_app.app.data_collection) == 4
75+
if 'load' in list(kwargs.keys())[0]:
76+
for i in (1, 2, 3):
77+
assert "1D Spectrum" in self.spec_app.app.data_collection[i].label
7178

7279
def test_load_multi_order_spectrum_list(self):
7380
assert len(self.spec_app.app.data_collection) == 1
7481
# now load ten spectral orders from a SpectrumList:
75-
self.spec_app.load_data(self.multi_order_spectrum_list)
82+
self.spec_app.load_data(self.multi_order_spectrum_list, sources='*')
7683
assert len(self.spec_app.app.data_collection) == 11
7784

7885
def test_mismatched_label_length(self):
@@ -417,20 +424,21 @@ def test_load_2d_flux(specviz_helper):
417424
# 1D Spectrum objects to load in Specviz.
418425
spec = Spectrum(spectral_axis=np.linspace(4000, 6000, 10)*u.Angstrom,
419426
flux=np.ones((4, 10))*u.Unit("1e-17 erg / (Angstrom cm2 s)"))
420-
specviz_helper.load_data(spec, data_label="test")
427+
428+
specviz_helper.load_data(spec, data_label="test", sources='*')
421429

422430
assert len(specviz_helper.app.data_collection) == 4
423-
assert specviz_helper.app.data_collection[0].label == "test_0"
431+
assert specviz_helper.app.data_collection[0].label == "test_index-0"
424432

425433
spec2 = Spectrum(spectral_axis=np.linspace(4000, 6000, 10)*u.Angstrom,
426434
flux=np.ones((2, 10))*u.Unit("1e-17 erg / (Angstrom cm2 s)"))
427435

428436
# Make sure 2D spectra in a SpectrumList also get split properly.
429437
spec_list = SpectrumList([spec, spec2])
430-
specviz_helper.load_data(spec_list, data_label="second test")
438+
specviz_helper.load_data(spec_list, data_label="second test", sources='*')
431439

432-
assert len(specviz_helper.app.data_collection) == 10
433-
assert specviz_helper.app.data_collection[-1].label == "second test_5"
440+
assert len(specviz_helper.app.data_collection) == 6
441+
assert specviz_helper.app.data_collection[-1].label == "second test_index-1"
434442

435443

436444
def test_plot_uncertainties(specviz_helper, spectrum1d):

0 commit comments

Comments
 (0)