-
Notifications
You must be signed in to change notification settings - Fork 74
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
Use get_data internally instead of get_data_from_viewer #2072
Changes from all commits
69279d7
7e21338
eb4c93e
b8b6ac4
094bb90
00915e1
87805e3
902c1c0
addc0f1
9bda36f
15d278a
9fc42b2
4c9f4c4
a0c36ae
9278a84
d799959
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -69,6 +69,8 @@ def test_spatial_spectral_mix(self): | |
# https://github.com/spacetelescope/jdaviz/issues/1584 | ||
with pytest.warns(UserWarning, match='Applying the value from the redshift slider'): | ||
spectral_subsets = self.cubeviz.specviz.get_spectra() | ||
assert list(spectral_subsets.keys()) == ['has_microns[FLUX]', 'Subset 1', 'Subset 2'], spectral_subsets # noqa | ||
assert list(spectral_subsets.keys()) == ['has_microns[FLUX]', | ||
'has_microns[FLUX] (Subset 1)', | ||
'has_microns[FLUX] (Subset 2)'], spectral_subsets # noqa | ||
Comment on lines
+72
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember if this paradigm for how data from |
||
for sp in spectral_subsets.values(): | ||
assert isinstance(sp, Spectrum1D) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -166,9 +166,7 @@ def _on_viewer_data_changed(self, msg=None): | |
|
||
label = msg.data.label | ||
try: | ||
viewer_data = self.app.get_data_from_viewer( | ||
self._default_spectrum_viewer_reference_name | ||
).get(label) | ||
viewer_data = self.app._jdaviz_helper.get_data(data_label=label) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh, I understand your desire to make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So is that an approval or a change request? 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha nothing to hold out on. Just a note |
||
except TypeError: | ||
warn_message = SnackbarMessage("Line list plugin could not retrieve data from viewer", | ||
sender=self, color="error") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense for now. But since
Spectrum1D
already has a mask attribute, we could(/should?) makespecviz.load_spectrum
load the mask so that you don't have to also index by the not-mask. Does the parser do this already/is this step necessary?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parser does load the mask (I believe you can show the mask and uncertainties, should be a toggle somewhere), but what Cami wanted to do was load in the data such that the data outside the subset is gone completely, which is what this does.