Skip to content

Issue 1119 changing spectral axis#1136

Open
astrofle wants to merge 3 commits into
release-1.1from
issue-1119-changing-spectral_axis
Open

Issue 1119 changing spectral axis#1136
astrofle wants to merge 3 commits into
release-1.1from
issue-1119-changing-spectral_axis

Conversation

@astrofle

Copy link
Copy Markdown
Collaborator

Adds Spectrum set_spectral_axis and with_spectral_axis_unit (overloads the specutils method).

set_spectral_axis modifies the units, frame or Doppler convention of the spectrum.
with_spectral_axis_unit adds the observer and target attributes back to the new spectrum object to make it compatible with dysh.spectra.Spectrum objects.

Example:

from dysh.spectra import Spectrum
s1 = Spectrum.fake_spectrum()
s1.set_spectral_axis(unit="km/s", doppler_convention="radio") # Changes the Spectrum in place.
s2 = s1.with_spectral_axis_unit("km/s", doppler_convention="optical") # Creates a new Spectrum.

astrofle added 2 commits June 23, 2026 15:38
set_spectral_axis modifies the units, frame or Doppler convention
of the spectrum. with_spectral_axis_unit adds the observer and
target attributes back to the new spectrum object to make it
compatible with dysh.spectra.Spectrum objects.
Adds set_spectral_axis to the user guide.
@astrofle astrofle requested a review from teuben June 23, 2026 19:50
@astrofle astrofle marked this pull request as ready for review June 24, 2026 01:51
@astrofle astrofle requested a review from mpound June 24, 2026 01:52

@mpound mpound left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The example fails for me because specutil.Spectrum.with_spectral_axis_unit uses velocity_convention as the keyword not doppler_convention so passing *args raises an exception.

Cell In[6], line 4
      2 s1 = Spectrum.fake_spectrum()
      3 s1.set_spectral_axis(unit="km/s", doppler_convention="radio") # Changes the Spectrum in place.
----> 4 s2 = s1.with_spectral_axis_unit("km/s", doppler_convention="optical") # Creates a new Spectrum.

File /bigdisk/src/dysh/src/dysh/spectra/spectrum.py:1101, in Spectrum.with_spectral_axis_unit(self, *args, **kwargs)
   1100 def with_spectral_axis_unit(self, *args, **kwargs):
-> 1101     spec = super().with_spectral_axis_unit(*args, **kwargs)
   1102     spec._observer = self.observer.copy()
   1103     spec._spectral_axis.observer = self.spectral_axis.observer.copy()

TypeError: OneDSpectrumMixin.with_spectral_axis_unit() got an unexpected keyword argument 'doppler_convention'

specutils.__version is '2.3.0'

The tests pass, though.

@astrofle astrofle force-pushed the issue-1119-changing-spectral_axis branch from 53b397b to 22f426f Compare June 25, 2026 03:48
@astrofle astrofle requested a review from mpound June 25, 2026 03:49
# Keep docstring empty to re-use the one from specutils.
# Keeping the specutils doc introduces yet another name
# for what we call `doppler_convention`, `velocity_convention`.
# This is a source of frustration...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Because specutils does not follow the astropy parameter name 'doppler_convention' in SpectralQuantity

@mpound mpound self-requested a review June 28, 2026 16:37

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Cell 6: sdfits["CTYPE1"][0] returns nan

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, sometimes notebook plots just hang but I don't belive that has anything to do with this PR.

@mpound mpound left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

switching to a spectrum with a km/s axis fails to set some metadata

2 participants