BUG: Fix specviz2d loader with ext keyword

[pllim]

Description

This pull request is to address user finding the app crashing when providing ext for Specviz2d loader.

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)? 🐱

[pllim]

This goes through a similar logic route as Cami's problematic case but does not give the exact traceback, probably because I am trying to fake it. But I don't feel like downloading even more data for the test suite and none of the existing remote data tests are triggering Cami's error. 🤷‍♀️

[camipacifici]

It loads the requested extension for NIRISS, NIRCam, and NIRSpec (thank you!), but when hovering on the 2D spectrum no information is reported in the top bar and plot options either does not update the 2D spectrum viewer or is extremely slow.

[pllim]

Hmm yeah, I guess fixing one case broke another because this parser is so overloaded. Thanks for checking. Turning back into draft.

[pllim]

The mouseover is something else. When you load jw01895001002_04101_00003_nrcalong_cal.fits with ext=8, its data.coords becomes a PaddedSpectrumWCS:

PaddedSpectrumWCS Transformation

This transformation has 2 pixel and 2 world dimensions

Array shape (Numpy order): None

Pixel Dim  Axis Name  Data size  Bounds
        0  None            None  None
        1  spatial         None  None

World Dim  Axis Name  Physical Type         Units
        0  None       custom:dimensionless  unknown
        1  Offset     None                  unknown

Correlation between pixel and world axes:

           Pixel Dim
World Dim    0    1
        0  yes   no
        1   no  yes

And calling data.coords.pixel_to_world(0, 0) with that WCS crashes:

UnitTypeError: SpectralCoord instances require units equivalent to '(Unit("Hz"), Unit("m"), Unit("J"), Unit("1 / m"), Unit("km / s"))', so cannot set it to ''.

[pllim]

When I try to construct the WCS from that file and extension directly as such, it gives me ValueError: Input WCS must have exactly one axis with spectral units, found 0. Now I am not sure if this is a software bug or problematic data.

from astropy import units as u
from astropy.io import fits
from astropy.wcs import WCS
from specutils import Spectrum1D

filename = "jw01895001002_04101_00003_nrcalong_cal.fits"
ext = 8
pf = fits.open(filename)
w = WCS(pf[ext].header)
flux = pf[ext].data * u.Unit(pf[ext].header["BUNIT"])
sp = Spectrum1D(flux=flux, wcs=w)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.94%. Comparing base (e56dfb3) to head (332b846).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2830      +/-   ##
==========================================
+ Coverage   88.92%   88.94%   +0.01%     
==========================================
  Files         111      111              
  Lines       17005    17006       +1     
==========================================
+ Hits        15122    15126       +4     
+ Misses       1883     1880       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[camipacifici]

Now the hovering works and plot options also works fine! When hovering on the 2D spectrum, the indicator does not get propagated to the 1D spectrum, although it gets propagated to the 2D spectrum when hovering on the 1D spectrum.
Everything is in pixels, which probably is expected? Mosviz is (or at least was) able to get the wavelength axis when plotting cal files, but maybe the wavelength was coming from the x1d file which here is not provided?
Thanks!

[pllim]

Re: #2830 (comment) -- Not sure what the expected behavior should be. I am just trying to fix the immediate bug here. I can ask other devs. Thanks for the quick feedback!

[kecnry]

Not sure what the expected behavior should be.

The hover is designed and expected to work in both directions, yes.

Screen.Recording.2024-04-25.at.7.58.42.AM.mov

[pllim]

To answer your questions from tag-up this morning.

Does Specviz2d Example Notebook still work with this patch?

@kecnry , yes.

Why does Mosviz show wavelength for cal file?

@camipacifici , that is because for NIRISS, it grabs wavelength from the corresponding WAVELENGTH extension. This seems to be very specific to NIRISS and the logic is buried deep inside Mosviz parser here:

jdaviz/jdaviz/configs/mosviz/plugins/parsers.py

Line 881 in 4423ceb

wav_hdus[i] = ('WAVELENGTH', temp[i].header['EXTVER'])

jdaviz/jdaviz/configs/mosviz/plugins/parsers.py

Lines 889 to 901 in 4423ceb

	# The wavelength is stored in a WAVELENGTH HDU. This is
	# a 2D array, but in order to be able to use Spectrum1D
	# we use the average wavelength for all image rows
	if data.shape[0] > data.shape[1]:
	# then the input data needs to be transposed, and wavelength
	# needs to be averaged over axis=1 instead of axis=0
	data = data.T
	wav = temp[wav_hdus[sci]].data.mean(axis=1) * u.micron
	else:
	wav = temp[wav_hdus[sci]].data.mean(axis=0) * u.micron
	spec2d = Spectrum1D(data * u.one, spectral_axis=wav, meta=meta)

This logic is inaccessible by Specviz2d.

[camipacifici]

Thank you, looks good to me.

[gibsongreen]

Specviz2d is working now and same with the hover, nice work! (I was curious and had to checkout the test data in the viewer for fun, reminded me of this painting in Chicago Cultural Center)

[pllim]

Thanks for the reviews!