spacetelescope / jwst

Python library for science observations from the James Webb Space Telescope
https://jwst-pipeline.readthedocs.io/en/latest/
Other
561 stars 167 forks source link

JP-3659: Fix for output WCS when first data set is empty #8562

Closed melanieclarke closed 3 months ago

melanieclarke commented 3 months ago

Resolves JP-3659

Closes #8561

Fix a small bug resulting in incorrect output spectral WCS when the first input data set contains only NaN.

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

melanieclarke commented 3 months ago

Spot-checking regression tests locally, I'm expecting these failures:

FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000031-crf] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000031-s2d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000031-x1d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[v000000049-s2d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[v000000049-x1d]

These MOS sources all have data that trigger the error condition: there is a small spectral scrap containing only NaN values used as the reference model for the output WCS. The b31 s2d is particularly egregious: because of the error, the good spectra do not align when resampled together, so the slit looks doubled in the output. After this change, the resampled data looks more sensible.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Project coverage is 59.51%. Comparing base (dba65ae) to head (28d8796). Report is 380 commits behind head on master.

Files with missing lines Patch % Lines
jwst/resample/resample_spec.py 92.30% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #8562 +/- ## ========================================== + Coverage 58.63% 59.51% +0.88% ========================================== Files 391 391 Lines 39081 39254 +173 ========================================== + Hits 22914 23362 +448 + Misses 16167 15892 -275 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

melanieclarke commented 3 months ago

Regression tests started here but later aborted: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1532/

My first fix didn't work for the reason I thought it did; trying something else.

melanieclarke commented 3 months ago

Okay, I think I have my fix the right way around now. These are the regtest failures I'm now expecting:

FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000030-crf] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000030-s2d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000030-x1d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000031-s2d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[b000000031-x1d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[v000000048-crf] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[v000000048-s2d] FAILED jwst/regtest/test_nirspec_mos_spec3.py::test_nirspec_mos_spec3[v000000048-x1d]

The b31 s2d that looked weirdly doubled to me was actually closer to correct than the stacked background in b30: it is doubled because it is a background for 2 dithers that don't overlap. The third dither for the same slit is marked as a virtual source instead (v49). The b30 s2d is now also appropriately doubled for the same reason; it is associated with virtual source v48.

Regression tests restarted here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1533/

melanieclarke commented 3 months ago

Regression test results look as expected, so I will take this out of draft.