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

Fix 2D smooth visibility bug by utilizing SpectralExtraction linking logic #2023

Merged
merged 7 commits into from
Feb 24, 2023
Merged

Fix 2D smooth visibility bug by utilizing SpectralExtraction linking logic #2023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ef07ad
Fix 2D smooth visibility bug by utilizing SpectralExtraction Linking …
duytnguyendtn Feb 23, 2023
30d24d5
Clarify testing comment
duytnguyendtn Feb 23, 2023
8d76ee9
Test mark.x against spectral_axis directly
duytnguyendtn Feb 23, 2023
a0def46
Increase default fixture size to accomodate for default trace backgro…
duytnguyendtn Feb 23, 2023
c2ce100
Remove unit custom options in fixture
duytnguyendtn Feb 24, 2023
87fc656
Changelog
duytnguyendtn Feb 24, 2023
68af3c0
Apply suggestions from code review
duytnguyendtn Feb 24, 2023
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: 2 additions & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Bug Fixes

* Loading valid data no longer emits JSON serialization warnings. [#2011]

* Fixed linking issue preventing smoothed spectrum from showing in Specviz2D. [#2023]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Fixed linking issue preventing smoothed spectrum from showing in Specviz2D. [#2023]
* Fixed linking issue preventing smoothed spectrum from showing in Specviz2d. [#2023]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Docs have it as capital D: https://jdaviz.readthedocs.io/en/latest/specviz2d/index.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, weird. I thought I remember we agreed to rebrand so that only first letter is capitalized (when we dropped the capital V). Also the class name has lowercase "d".

jdaviz/jdaviz/configs/specviz2d/helper.py

Line 12 in 71a514c

class Specviz2d(ConfigHelper, LineListMixin):

Now I am confused. But I guess this discussion should not block merge. I already approved. Thanks for your patience!


Cubeviz
^^^^^^^

Expand Down
6 changes: 5 additions & 1 deletion jdaviz/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,11 @@ def _link_new_data(self, reference_data=None, data_to_be_linked=None):
LinkSame(linked_data.components[0], ref_data.components[1])]
dc.add_link(links)
return
elif linked_data.meta.get('Plugin', None) == 'SpectralExtraction':
elif (linked_data.meta.get('Plugin', None) == 'SpectralExtraction' or
(linked_data.meta.get('Plugin', None) == ('GaussianSmooth') and
linked_data.ndim < 3 and # Cube linking requires special logic. See below
ref_data.ndim < 3)
):
links = [LinkSame(linked_data.components[0], ref_data.components[0]),
LinkSame(linked_data.components[1], ref_data.components[1])]
dc.add_link(links)
Expand Down
28 changes: 28 additions & 0 deletions jdaviz/configs/default/plugins/gaussian_smooth/tests/test_gaussian_smooth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import numpy as np
import pytest
from astropy.utils.exceptions import AstropyUserWarning
from specutils import Spectrum1D
Expand Down Expand Up @@ -157,3 +158,30 @@ def test_spectrum1d_smooth(specviz_helper, spectrum1d):
{'event': 'mousemove', 'domain': {'x': 5500, 'y': 120}})
assert label_mouseover.as_text() == ('', '', '')
assert label_mouseover.icon == ''


def test_spectrum2d_smooth(specviz2d_helper, spectrum2d):
data_label = 'test'
dc = specviz2d_helper.app.data_collection
specviz2d_helper.load_data(spectrum_2d=spectrum2d, spectrum_2d_label=data_label)

gs_plugin = specviz2d_helper.plugins['Gaussian Smooth']

# The Autocollapsed spectrum is given the label of "Spectrum 1D by default"
smooth_source_dataset = "Spectrum 1D"
gs_plugin.dataset = smooth_source_dataset
test_stddev_level = 100.0
duytnguyendtn marked this conversation as resolved.
Show resolved Hide resolved
gs_plugin.stddev = test_stddev_level
gs_plugin.smooth()

assert len(dc) == 3
assert dc[2].label == f'{smooth_source_dataset} smooth stddev-{test_stddev_level}'

# Ensure all marks were created properly (i.e. not in their initialized state)
# [0,1] is the default (initialization) value for the marks
marks = specviz2d_helper.app.get_viewer('spectrum-viewer').native_marks

assert len(marks) == 2
for mark in marks:
np.testing.assert_allclose(mark.x, spectrum2d.spectral_axis.value)
assert not np.array_equal(mark.y, [0, 1])
12 changes: 12 additions & 0 deletions jdaviz/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ def mos_spectrum1d():
return Spectrum1D(spectral_axis=spec_axis, flux=flux)


@pytest.fixture
def spectrum2d():
'''
A simple 2D Spectrum1D with a center "trace" array rising from 0 to 10
with two "zero array" buffers above and below
'''
data = np.zeros((5, 10))
data[3] = np.arange(10)

return Spectrum1D(flux=data*u.MJy, spectral_axis=data[3]*u.um)


@pytest.fixture
def mos_spectrum2d():
'''
Expand Down