JP-3631: remove direct setting of self.skip within calibration steps

CHANGES.rst

@@ -1,7 +1,24 @@
 1.15.1 (unreleased)
 ===================
 
--
+cube_build
+----------
+
+- Removed direct setting of the ``self.skip`` attribute from within the step
+  itself. [#8600]
+
+stpipe
+------
+
+- Removed setting of the `self.skip` attribute in the `record_step_status()` function;
+  added a `query_step_status()` function to use as an alternative to checking 
+  `self.skip`. [#8600]
+
+tweakreg
+--------
+
+- Removed direct setting of the ``self.skip`` attribute from within the step
+  itself. [#8600]
 
 1.15.0 (2024-06-26)
 ===================

jwst/cube_build/cube_build_step.py

@@ -3,7 +3,7 @@
 
 from jwst.datamodels import ModelContainer
 
-from ..stpipe import Step
+from ..stpipe import Step, record_step_status
 from . import cube_build
 from . import ifu_cube
 from . import data_types
@@ -396,14 +396,13 @@
                 if key in cube.meta.wcsinfo.instance:
                     del cube.meta.wcsinfo.instance[key]
         if status_cube == 1:
-            self.skip = True
+            record_step_status(cube_container, "cube_build", success=False)
+        else:
+            record_step_status(cube_container, "cube_build", success=True)
 
         t1 = time.time()
         self.log.debug(f'Time to build all cubes {t1-t0}')
 
-        if status_cube == 1:
-            self.skip = True
-
         input_table.close()
         return cube_container
 # ******************************************************************************

jwst/master_background/master_background_mos_step.py

@@ -1,5 +1,6 @@
 """Master Background Pipeline for applying Master Background to NIRSpec MOS data"""
 from stpipe.step import preserve_step_pars
+from jwst.stpipe import record_step_status, query_step_status
 
 from stdatamodels.jwst import datamodels
 
@@ -106,9 +107,9 @@
             # If some type of background processing had already been done. Abort.
             # UNLESS forcing is enacted.
             if not self.force_subtract and \
-               'COMPLETE' in [data_model.meta.cal_step.back_sub, data_model.meta.cal_step.master_background]:
+               'COMPLETE' in [query_step_status(data_model, "back_sub"), query_step_status(data_model, "master_background")]:
                 self.log.info('Background subtraction has already occurred. Skipping.')
-                self.record_step_status(data, 'master_background', False)
+                record_step_status(data, 'master_background', success=False)
                 return data
 
             if self.user_background:
@@ -123,11 +124,11 @@
                 num_bkg, num_src = self._classify_slits(data_model)
                 if num_bkg == 0:
                     self.log.warning('No background slits available for creating master background. Skipping')
-                    self.record_step_status(data, 'master_background', False)
+                    record_step_status(data, 'master_background', False)
                     return data
                 elif num_src == 0:
                     self.log.warning('No source slits for applying master background. Skipping')
-                    self.record_step_status(data, 'master_background', False)
+                    record_step_status(data, 'master_background', False)
                     return data
 
                 self.log.info('Calculating master background')
@@ -136,14 +137,14 @@
             # Check that a master background was actually determined.
             if master_background is None:
                 self.log.info('No master background could be calculated. Skipping.')
-                self.record_step_status(data, 'master_background', False)
+                record_step_status(data, 'master_background', False)
                 return data
 
             # Now apply the de-calibrated background to the original science
             result = nirspec_utils.apply_master_background(data_model, mb_multislit, inverse=self.inverse)
 
             # Mark as completed and setup return data
-            self.record_step_status(result, 'master_background', True)
+            record_step_status(result, 'master_background', True)
             self.correction_pars = {
                 'masterbkg_1d': master_background,
                 'masterbkg_2d': mb_multislit

jwst/master_background/master_background_step.py

@@ -4,6 +4,7 @@
 from stdatamodels.jwst import datamodels
 
 from jwst.datamodels import ModelContainer
+from jwst.stpipe import record_step_status
 
 from ..stpipe import Step
 from ..combine_1d.combine1d import combine_1d_spectra
@@ -63,7 +64,7 @@
             # First check if we should even do the subtraction.  If not, bail.
             if not self._do_sub:
                 result = input_data.copy()
-                self.record_step_status(result, 'master_background', success=False)
+                record_step_status(result, 'master_background', success=False)
                 return result
 
             # Check that data is a supported datamodel. If not, bail.
@@ -78,7 +79,7 @@
                     "Input %s of type %s cannot be handled.  Step skipped.",
                     input, type(input)
                 )
-                self.record_step_status(result, 'master_background', success=False)
+                record_step_status(result, 'master_background', success=False)
                 return result
 
             # If user-supplied master background, subtract it
@@ -159,15 +160,15 @@
                         "Input %s of type %s cannot be handled without user-supplied background.  Step skipped.",
                         input, type(input)
                     )
-                    self.record_step_status(result, 'master_background', success=False)
+                    record_step_status(result, 'master_background', success=False)
                     return result
 
                 # Save the computed background if requested by user
                 if self.save_background:
                     self.save_model(master_background, suffix='masterbg1d', force=True, asn_id=asn_id)
                     self.save_model(background_2d_collection, suffix='masterbg2d', force=True, asn_id=asn_id)
 
-            self.record_step_status(result, 'master_background', success=True)
+            record_step_status(result, 'master_background', success=True)
 
         return result
 

jwst/master_background/nirspec_utils.py

@@ -254,7 +254,8 @@ def is_background_msa_slit(slit):
     bool
         True if the slit is background; False if it is not.
     """
-    if "BKG" in str(slit.source_name).upper():
+    name = str(slit.source_name).upper()
+    if ("BKG" in name) or ("BACKGROUND" in name):
         return True
     else:
         return False

jwst/master_background/tests/test_nirspec_corrections.py

@@ -59,7 +59,7 @@ def test_fs_correction():
 
 @pytest.mark.parametrize('name,status',
                          [('BKG101', True), ('bkg101', True),
-                          ('background_101', False), ('101', False),
+                          ('background_101', True), ('101', False),
                           (None, False)])
 def test_is_background(name, status):
     """Test check for background slit."""

jwst/pipeline/calwebb_spec2.py

@@ -5,6 +5,7 @@
 import numpy as np
 
 from stdatamodels.jwst import datamodels
+from jwst.stpipe import query_step_status
 
 from ..assign_wcs.util import NoDataOnDetectorError
 from ..lib.exposure_types import is_nrs_ifu_flatlamp, is_nrs_ifu_linelamp, is_nrs_slit_linelamp
@@ -332,7 +333,7 @@ def process_exposure_product(
             # as DO_NOT_USE or NON_SCIENCE.
             resampled = self.pixel_replace(resampled)
             resampled = self.cube_build(resampled)
-            if not self.cube_build.skip:
+            if query_step_status(resampled, "cube_build") == 'COMPLETE':
                 self.save_model(resampled[0], suffix='s3d')
         elif exp_type in ['MIR_LRS-SLITLESS']:
             resampled = calibrated.copy()
@@ -346,7 +347,7 @@ def process_exposure_product(
             # as DO_NOT_USE or NON_SCIENCE.
             resampled = self.pixel_replace(resampled)
         # Extract a 1D spectrum from the 2D/3D data
-        if exp_type in ['MIR_MRS', 'NRS_IFU'] and self.cube_build.skip:
+        if exp_type in ['MIR_MRS', 'NRS_IFU'] and query_step_status(resampled, "cube_build") == 'SKIPPED':
             # Skip extract_1d for IFU modes where no cube was built
             self.extract_1d.skip = True
 

jwst/pipeline/calwebb_spec3.py

@@ -6,6 +6,7 @@
 from stdatamodels.jwst import datamodels
 
 from jwst.datamodels import SourceModelContainer
+from jwst.stpipe import query_step_status
 
 from ..associations.lib.rules_level3_base import format_product
 from ..exp_to_source import multislit_to_container
@@ -148,7 +149,7 @@
 
             # If the step is skipped, do the container splitting that
             # would've been done in master_background
-            if self.master_background.skip:
+            if query_step_status(source_models, "master_background") == 'SKIPPED':
                 source_models, bkg_models = split_container(input_models)
                 # we don't need the background members
                 bkg_models.close()

jwst/pixel_replace/pixel_replace_step.py

@@ -2,6 +2,7 @@
 from functools import partial
 
 from ..stpipe import Step
+from jwst.stpipe import record_step_status
 from .. import datamodels
 from .pixel_replace import PixelReplacement
 
@@ -107,7 +108,7 @@
                         replacement = PixelReplacement(model, **pars)
                         replacement.replace()
                         output_model[i] = replacement.output
-                        self.record_step_status(output_model[i], 'pixel_replace', success=True)
+                        record_step_status(output_model[i], 'pixel_replace', success=True)
                 return output_model
             # ________________________________________
             # calewbb_spec2 case - single input model
@@ -117,6 +118,6 @@
                 result = input_model.copy()
                 replacement = PixelReplacement(result, **pars)
                 replacement.replace()
-                self.record_step_status(replacement.output, 'pixel_replace', success=True)
+                record_step_status(replacement.output, 'pixel_replace', success=True)
                 result = replacement.output
                 return result

jwst/stpipe/__init__.py

@@ -1,4 +1,5 @@
 from .core import JwstStep as Step, JwstPipeline as Pipeline
+from .utilities import record_step_status, query_step_status
 
 
-__all__ = ['Step', 'Pipeline']
+__all__ = ['Step', 'Pipeline', 'record_step_status', 'query_step_status']

jwst/stpipe/core.py

@@ -1,7 +1,6 @@
 """
 JWST-specific Step and Pipeline base classes.
 """
-from collections.abc import Sequence
 from stdatamodels.jwst.datamodels import JwstDataModel
 from stdatamodels.jwst import datamodels
 
@@ -89,33 +88,6 @@ def finalize_result(self, result, reference_files_used):
                 if self.parent is None:
                     log.info(f"Results used CRDS context: {result.meta.ref_file.crds.context_used}")
 
-    def record_step_status(self, datamodel, cal_step, success=True):
-        """Record whether or not a step completed in meta.cal_step
-
-        Parameters
-        ----------
-        datamodel : `~jwst.datamodels.JwstDataModel` instance
-            This is the datamodel or container of datamodels to modify in place
-
-        cal_step : str
-            The attribute in meta.cal_step for recording the status of the step
-
-        success : bool
-            If True, then 'COMPLETE' is recorded.  If False, then 'SKIPPED'
-        """
-        if success:
-            status = 'COMPLETE'
-        else:
-            status = 'SKIPPED'
-            self.skip = True
-
-        if isinstance(datamodel, Sequence):
-            for model in datamodel:
-                model.meta.cal_step._instance[cal_step] = status
-        else:
-            datamodel.meta.cal_step._instance[cal_step] = status
-
-        # TODO: standardize cal_step naming to point to the official step name
 
     def remove_suffix(self, name):
         return remove_suffix(name)

jwst/stpipe/tests/test_utilities.py

@@ -1,9 +1,12 @@
 """Test utility funcs"""
+import pytest
 from stpipe.utilities import resolve_step_class_alias
+from jwst.stpipe import record_step_status, query_step_status
 
-from jwst.stpipe.utilities import all_steps
+from jwst.stpipe.utilities import all_steps, NOT_SET
 import jwst.pipeline
 import jwst.step
+from jwst import datamodels as dm
 
 
 # All steps available to users should be represented in one
@@ -25,3 +28,33 @@ def test_resolve_step_class_alias():
         if pipeline_class.class_alias is not None:
             assert resolve_step_class_alias(pipeline_class.class_alias) == full_class_name
         assert resolve_step_class_alias(full_class_name) == full_class_name
+
+
+def test_record_query_step_status():
+
+    # single model case
+    model = dm.MultiSpecModel()
+    record_step_status(model, 'test_step', success=True)
+    assert query_step_status(model, 'test_step') == 'COMPLETE'
+
+    # modelcontainer case where all skipped
+    model2 = dm.ModelContainer()
+    model2.append(dm.MultiSpecModel())
+    model2.append(dm.MultiSpecModel())
+    record_step_status(model2, 'test_step', success=False)
+    assert model2[0].meta.cal_step._instance['test_step'] == 'SKIPPED'
+    assert model2[1].meta.cal_step._instance['test_step'] == 'SKIPPED'
+    assert query_step_status(model2, 'test_step') == 'SKIPPED'
+
+    # modelcontainer case with at least one complete
+    model2.append(dm.MultiSpecModel())
+    model2[2].meta.cal_step._instance['test_step'] = 'COMPLETE'
+    assert query_step_status(model2, 'test_step') == 'COMPLETE'
+
+    model2[2].meta.cal_step._instance['test_step'] = 'UNRECOGNIZED'
+    with pytest.raises(ValueError):
+        query_step_status(model2, 'test_step')
+
+    # test query not set
+    model3 = dm.MultiSpecModel()
+    assert query_step_status(model3, 'test_step') == NOT_SET