Fix bug marking background slits as sources in jwst.assign_wcs.nirspec.py #8494
Conversation
Previously the code in `jwst.assign_wcs.nirspec.py` can lead to namespace clashes, where the temporary `source_id`s assigned to the background slits are checked against the full source catalog list and incorrectly match to real sources in the full source catalog, instead of failing the try/except Index error in lines 702-712 to return unique source names of `background_{slitlet_id}`. This leads to incorrect targets marked as being assigned slits in the WCS assignment, and background slits extracted as if they were sources in the full reduction pipeline.
The source catalog (`msa_file[("SOURCE_INFO", 1)].data`) is originally defined in the APT, and often includes many more objects than are assigned slits (`msa_file[('SHUTTER_INFO', 1)]`) in a given MSA configuration. Background slits can therefore be assigned `source_id`s that correspond to actual objects in the source catalog.
The desired behavior, that background slits are re-identified and assigned source information appropriately (lines 707-712), can be achieved with the fix of switching from a try/except IndexError to an explicit check if the object `source_id` is less than the identified `max_source_id`.
|
@sedonaprice The whole scheme by which background and virtual MOS slits are handled is being completely redone in #8442, in order to avoid just these kinds of problems and to accommodate virtual slits. In the new scheme, background slits are assigned a source_id equal to their slitlet_id, but all tracking/sorting/grouping of source data in downstream steps will now be based on the source_name (instead of source_id number) and the source_name fields will distinguish between "source", "background", and "virtual" slits. So there should no longer be any collisions or confusion. I suggest waiting till #8442 is merged and then see if this is still a problem. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8494 +/- ##
==========================================
- Coverage 57.93% 57.93% -0.01%
==========================================
Files 387 387
Lines 38839 38851 +12
==========================================
+ Hits 22502 22507 +5
- Misses 16337 16344 +7 ☔ View full report in Codecov by Sentry. |
This PR addresses a bug where background slits are incorrectly marked as sources in
jwst.assign_wcs.nirspec.py::get_open_msa_slits, leading them to be marked as sources that were not assigned to slits but were in the source catalog. These incorrectly marked background slits are then extracted as targets.Full explanation:
Previously the code in
jwst.assign_wcs.nirspec.py::get_open_msa_slitscan lead to namespace clashes, where the temporarysource_ids assigned to the background slits are checked against the full source catalog list and incorrectly match to real sources in the full source catalog, instead of failing the try/except Index error in lines 702-712 to return unique source names ofbackground_{slitlet_id}. This leads to incorrect targets marked as being assigned slits in the WCS assignment, and background slits extracted as if they were sources in the full reduction pipeline.The source catalog (
msa_file[("SOURCE_INFO", 1)].data) is originally defined in the APT, and often includes many more objects than are assigned slits (msa_file[('SHUTTER_INFO', 1)]) in a given MSA configuration. Background slits can therefore be assignedsource_ids that correspond to actual objects in the source catalog.The desired behavior, that background slits are re-identified and assigned source information appropriately (lines 707-712), can be achieved with the fix of switching from a try/except IndexError to an explicit check if the object
source_idis less than the identifiedmax_source_id.Checklist for PR authors (skip items if you don't have permissions or they are not applicable)
CHANGES.rstwithin the relevant release sectionHow to run regression tests on a PR