Wrap "process_slit" in try/except for "extract_2d.nirspec.nrs_extract2d"

[gbrammer]

This PR addresses behavior in extract_2d.nirspec.nrs_extract2d, which can fail if no valid pixels are found in a cutout that are then used to define the slit range of the bounding box.

This behavior doesn't seem to occur in normal pipeline processing, but I'm working on tests to change (i.e., extend) the wavelength bounds of spectral cutouts. Nearly all pipeline functions seem to work perfectly fine after simply changing the values in the wavelengthrange reference file, but occasional problems can occur for PRISM slitlets where the padded extended cutout overlaps a particular detector but where there are no resulting valid unpadded pixels used to compute the slit coordinates of the bounding box in jwst.assign_wcs.nirspec.compute_bounding_box.

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
• Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
  • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • start a regression test and include a link to the running job (click here for instructions)
    • Do truth files need to be updated ("okified")?
      • after the reviewer has approved these changes, run okify_regtests to update the truth files
• if a JIRA ticket exists, make sure it is resolved properly

news fragment change types...

• changes/<PR#>.general.rst: infrastructure or miscellaneous change
• changes/<PR#>.docs.rst
• changes/<PR#>.stpipe.rst
• changes/<PR#>.datamodels.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.fits_generator.rst
• changes/<PR#>.set_telescope_pointing.rst
• changes/<PR#>.pipeline.rst

stage 1

• changes/<PR#>.group_scale.rst
• changes/<PR#>.dq_init.rst
• changes/<PR#>.emicorr.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.ipc.rst
• changes/<PR#>.firstframe.rst
• changes/<PR#>.lastframe.rst
• changes/<PR#>.reset.rst
• changes/<PR#>.superbias.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.rscd.rst
• changes/<PR#>.persistence.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.charge_migration.rst
• changes/<PR#>.jump.rst
• changes/<PR#>.clean_flicker_noise.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.gain_scale.rst

stage 2

• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.badpix_selfcal.rst
• changes/<PR#>.msaflagopen.rst
• changes/<PR#>.nsclean.rst
• changes/<PR#>.imprint.rst
• changes/<PR#>.background.rst
• changes/<PR#>.extract_2d.rst
• changes/<PR#>.master_background.rst
• changes/<PR#>.wavecorr.rst
• changes/<PR#>.srctype.rst
• changes/<PR#>.straylight.rst
• changes/<PR#>.wfss_contam.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.fringe.rst
• changes/<PR#>.pathloss.rst
• changes/<PR#>.barshadow.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.pixel_replace.rst
• changes/<PR#>.resample_spec.rst
• changes/<PR#>.residual_fringe.rst
• changes/<PR#>.cube_build.rst
• changes/<PR#>.extract_1d.rst
• changes/<PR#>.resample.rst

stage 3

• changes/<PR#>.assign_mtwcs.rst
• changes/<PR#>.mrs_imatch.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.exp_to_source.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.tso_photometry.rst
• changes/<PR#>.stack_refs.rst
• changes/<PR#>.align_refs.rst
• changes/<PR#>.klip.rst
• changes/<PR#>.spectral_leak.rst
• changes/<PR#>.source_catalog.rst
• changes/<PR#>.combine_1d.rst
• changes/<PR#>.ami.rst

other

• changes/<PR#>.wfs_combine.rst
• changes/<PR#>.white_light.rst
• changes/<PR#>.cube_skymatch.rst
• changes/<PR#>.engdb_tools.rst
• changes/<PR#>.guider_cds.rst

[melanieclarke]

Hello, and thanks for sending this in! The try/except change to extract_2d sounds pretty benign, but I'm not sure why there are changes to set_telescope_pointing in this PR as well. Were those included in error?

[gbrammer]

Oh, I guess that was an earlier commit to my fork where I had implemented providing an explicit MAST_TOKEN to various functions there. Is it possible to just remove the changes to that file from this PR and just keep the changes to extrac2d? I'm not sure I've ever done that before on GH.

[melanieclarke]

Is it possible to just remove the changes to that file from this PR and just keep the changes to extrac2d?

Yes, that's possible! Just make the changes in your own local version and push them up to the same branch - the PR will automatically update. If you make the changes via rebasing or editing old commits, you may need to add a force-push flag when you push up the changes to your fork (git push -f).

[gbrammer]

Thanks for the help. I've removed the other change so now this PR is only for the small update to extract_2d.

[codecov]

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 64.52%. Comparing base (72cb6f0) to head (e047d49).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
jwst/extract_2d/nirspec.py	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8964      +/-   ##
==========================================
- Coverage   64.52%   64.52%   -0.01%     
==========================================
  Files         375      375              
  Lines       38739    38743       +4     
==========================================
+ Hits        24997    24998       +1     
- Misses      13742    13745       +3     

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

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[melanieclarke]

This needs a change log entry - please add a one-line summary to a file called 8964.extract_2d.rst, in the changes directory.

One quick suggestion for formatting the warning string, but other than that, I think this looks fine. I'll run regression tests. @hayescr - any objections from NIRSpec?

[melanieclarke]

This would be a little simpler formatted as an f-string:

Suggested change

	'process_slit failed for slit/subarray {0}, '.format(slit.name)
	+ 'probably because the cutout has no valid pixels'
	f'process_slit failed for slit {slit.name}, '
	f'probably because the cutout has no valid pixels'

[hayescr]

This looks reasonable to me as well, though the only thought I have is whether it's possible to do a more specific catch that we could be doing when building the cutouts (e.g., checking whether the unpadded pixels fall on a detector).

I'd be happy to take a look if you could let me know what data and PRISM wavelength range you were running into these errors @gbrammer.

[gbrammer]

@melanieclarke

• I added changes/8964.extract_2d.rst.
• I also prefer f-strings but I had written it with format to match the other comments elsewhere in the same file.

@hayescr

• I agree that catching the bug at the source would be preferable, this was just the simplest fix I could implement. The bug comes when assign_wcs.nirspec.compute_bounding_box tries to compute y_range.min() in cases where y_range.min() is a null array after masking all NaN pixels.
• One offending dataset would be jw02750002001_03101_00001_nrs1_rate.fits with PRISM_CLEAR: 0.5, 5.7 in the wavelengthrange file.

[melanieclarke]

@gbrammer - just an update: we're still looking into this, but need to move it to the back burner for a week or two. We'll get back to you when we have availability again.