Set FAILED status steps that did not succeed

melanieclarke · melanieclarke · commit ac4d15e7a143 · 2026-04-29T13:40:52.000-04:00
diff --git a/jwst/assign_mtwcs/tests/test_mtwcs.py b/jwst/assign_mtwcs/tests/test_mtwcs.py
@@ -107,7 +107,7 @@ def test_output_is_not_input(monkeypatch, success):
     that performance is the most important thing and extra copies are
     not desired.
     """
-    # Mock a failure in the ModelLibrary init, to exercise the "skipped" condition
+    # Mock a failure in the ModelLibrary init, to exercise the "failed" condition
     if not success:
 
         def raise_error(*args, **kwargs):
@@ -127,7 +127,7 @@ def raise_error(*args, **kwargs):
                 if success:
                     assert im.meta.cal_step.assign_mtwcs == "COMPLETE"
                 else:
-                    assert im.meta.cal_step.assign_mtwcs == "SKIPPED"
+                    assert im.meta.cal_step.assign_mtwcs == "FAILED"
                 assert im is not input_im
                 assert input_im.meta.cal_step.assign_mtwcs is None
 
diff --git a/jwst/master_background/master_background_mos_step.py b/jwst/master_background/master_background_mos_step.py
@@ -120,7 +120,7 @@ def process(self, data):
             output_model.meta.cal_step.master_background,
         ]:
             log.info("Background subtraction has already occurred. Skipping.")
-            record_step_status(output_model, "master_background", success=False)
+            record_step_status(output_model, "master_background", status="SKIPPED")
             return output_model
 
         bkg_x1d_spectra = None
@@ -143,11 +143,11 @@ def process(self, data):
                 log.warning(
                     "No background slits available for creating master background. Skipping"
                 )
-                record_step_status(output_model, "master_background", False)
+                record_step_status(output_model, "master_background", status="SKIPPED")
                 return output_model
             elif num_src == 0:
                 log.warning("No source slits for applying master background. Skipping")
-                record_step_status(output_model, "master_background", False)
+                record_step_status(output_model, "master_background", status="SKIPPED")
                 return output_model
 
             log.info("Calculating master background")
@@ -158,7 +158,7 @@ def process(self, data):
         # Check that a master background was actually determined.
         if master_background is None:
             log.info("No master background could be calculated. Skipping.")
-            record_step_status(output_model, "master_background", False)
+            record_step_status(output_model, "master_background", status="FAILED")
             return output_model
 
         # Now apply the de-calibrated background to the original science
@@ -167,7 +167,7 @@ def process(self, data):
         )
 
         # Mark as completed and setup return data
-        record_step_status(result, "master_background", True)
+        record_step_status(result, "master_background", status="COMPLETE")
         self.correction_pars = {"masterbkg_1d": master_background, "masterbkg_2d": mb_multislit}
         if self.save_background:
             self.save_model(master_background, suffix="masterbg1d", force=True)
diff --git a/jwst/master_background/master_background_step.py b/jwst/master_background/master_background_step.py
@@ -53,7 +53,7 @@ def process(self, input_data):
 
         # First check if we should even do the subtraction.  If not, bail.
         if not self._do_sub(output_model):
-            record_step_status(output_model, "master_background", success=False)
+            record_step_status(output_model, "master_background", status="SKIPPED")
             return output_model
 
         # Check that data is a supported datamodel. If not, bail.
@@ -70,7 +70,7 @@ def process(self, input_data):
             log.warning(
                 f"Input {input_data} of type {type(output_model)} cannot be handled.  Step skipped."
             )
-            record_step_status(output_model, "master_background", success=False)
+            record_step_status(output_model, "master_background", status="SKIPPED")
             return output_model
 
         # If user-supplied master background, subtract it
@@ -114,7 +114,7 @@ def process(self, input_data):
                     f"Input {input_data} of type {type(output_model)} cannot be "
                     "handled without user-supplied background.  Step skipped."
                 )
-                record_step_status(output_model, "master_background", success=False)
+                record_step_status(output_model, "master_background", status="SKIPPED")
                 return output_model
 
             result, background_data = split_container(output_model)
@@ -124,7 +124,7 @@ def process(self, input_data):
                     "and no user-supplied background provided.  Skipping step."
                 )
                 log.warning(msg)
-                record_step_status(output_model, "master_background", success=False)
+                record_step_status(output_model, "master_background", status="SKIPPED")
                 return output_model
             asn_id = result.asn_table["asn_id"]
 
@@ -189,7 +189,7 @@ def process(self, input_data):
                     background_2d_collection, suffix="masterbg2d", force=True, asn_id=asn_id
                 )
 
-        record_step_status(result, "master_background", success=True)
+        record_step_status(result, "master_background", status="COMPLETE")
 
         # Clean up intermediate background models
         background_2d_collection.close()
diff --git a/jwst/master_background/tests/test_master_background_mos.py b/jwst/master_background/tests/test_master_background_mos.py
@@ -265,9 +265,9 @@ def test_skip_no_master_bg(monkeypatch, nirspec_msa_extracted2d):
     # mock a failure in master bg creation
     monkeypatch.setattr(step, "_extend_bg_slits", lambda *args, **kwargs: None)
 
-    # Step is skipped
+    # Step is marked FAILED
     result = step.run(model)
-    assert result.meta.cal_step.master_background == "SKIPPED"
+    assert result.meta.cal_step.master_background == "FAILED"
 
     # Input is not modified
     assert result is not model
diff --git a/jwst/outlier_detection/outlier_detection_step.py b/jwst/outlier_detection/outlier_detection_step.py
@@ -171,7 +171,7 @@ def process(self, input_data):
             log.error(f"Outlier detection failed for unknown/unsupported mode: {mode}")
             record_step_status(result_models, "outlier_detection", False)
 
-        if query_step_status(result_models, "outlier_detection") != "SKIPPED":
+        if query_step_status(result_models, "outlier_detection") not in ("SKIPPED", "FAILED"):
             record_step_status(result_models, "outlier_detection", True)
         return result_models
 
diff --git a/jwst/outlier_detection/tests/test_ifu.py b/jwst/outlier_detection/tests/test_ifu.py
@@ -9,8 +9,8 @@ def test_ifu_one_exposure(miri_ifu_rate):
     input_model = miri_ifu_rate
     result = OutlierDetectionStep.call([input_model])
 
-    # Step is skipped
-    assert result[0].meta.cal_step.outlier_detection == "SKIPPED"
+    # Step is failed
+    assert result[0].meta.cal_step.outlier_detection == "FAILED"
 
     # Input is not modified
     assert result[0] is not input_model
diff --git a/jwst/outlier_detection/tests/test_imaging.py b/jwst/outlier_detection/tests/test_imaging.py
@@ -274,7 +274,7 @@ def test_skip_one_exposure_imaging():
     result = step.run(input_models)
 
     # Step is skipped
-    assert query_step_status(result, "outlier_detection") == "SKIPPED"
+    assert query_step_status(result, "outlier_detection") == "FAILED"
 
     # Input is not modified
     assert result is not model
diff --git a/jwst/outlier_detection/tests/test_outlier_detection.py b/jwst/outlier_detection/tests/test_outlier_detection.py
@@ -117,7 +117,7 @@ def test_guess_mode_assigned(caplog, mode):
     step.mode = mode
     result = step.run(input_model)
 
-    assert result.meta.cal_step.outlier_detection == "SKIPPED"
+    assert result.meta.cal_step.outlier_detection == "FAILED"
     assert isinstance(result, datamodels.ImageModel)
 
     # If mode was unrecognized, error message is issued
@@ -137,8 +137,8 @@ def test_skip_unknown_mode_file(tmp_path, caplog):
     input_model.save(input_file)
     result = OutlierDetectionStep.call(input_file)
 
-    # Step is skipped with an error message
-    assert result.meta.cal_step.outlier_detection == "SKIPPED"
+    # Step is failed with an error message
+    assert result.meta.cal_step.outlier_detection == "FAILED"
     assert isinstance(result, datamodels.ImageModel)
     assert "ERROR" in caplog.text
 
@@ -152,8 +152,8 @@ def test_skip_unknown_mode_image_model():
     input_model = datamodels.ImageModel()
     result = OutlierDetectionStep.call(input_model)
 
-    # Step is skipped
-    assert result.meta.cal_step.outlier_detection == "SKIPPED"
+    # Step is failed
+    assert result.meta.cal_step.outlier_detection == "FAILED"
 
     # Input is not modified
     assert result is not input_model
@@ -165,8 +165,8 @@ def test_skip_unknown_mode_container():
     container = ModelContainer([input_model])
     result = OutlierDetectionStep.call(container)
 
-    # Step is skipped
-    assert result[0].meta.cal_step.outlier_detection == "SKIPPED"
+    # Step is failed
+    assert result[0].meta.cal_step.outlier_detection == "FAILED"
 
     # Input is not modified
     assert result[0] is not input_model
diff --git a/jwst/outlier_detection/tests/test_spec.py b/jwst/outlier_detection/tests/test_spec.py
@@ -122,8 +122,8 @@ def test_skip_one_exposure_spec():
     # it should skip and return a copy of the input with the status set
     result = step.run(input_models)
 
-    # Step is skipped
-    assert result[0].meta.cal_step.outlier_detection == "SKIPPED"
+    # Step is failed
+    assert result[0].meta.cal_step.outlier_detection == "FAILED"
 
     # Input is not modified
     assert result[0] is not model
diff --git a/jwst/pixel_replace/pixel_replace_step.py b/jwst/pixel_replace/pixel_replace_step.py
@@ -57,7 +57,7 @@ def process(self, input_data):
             log.error(f"Input is of type {str(output_model)} for which")
             log.error("pixel_replace does not have an algorithm.")
             log.error("Pixel replacement will be skipped.")
-            output_model.meta.cal_step.pixel_replace = "SKIPPED"
+            record_step_status(output_model, "pixel_replace", success=False)
             return output_model
 
         pars = {
diff --git a/jwst/pixel_replace/tests/test_pixel_replace.py b/jwst/pixel_replace/tests/test_pixel_replace.py
@@ -360,8 +360,8 @@ def test_skip_unexpected_type():
     bad_model = datamodels.RampModel()
     result = PixelReplaceStep.call(bad_model)
 
-    # Step is skipped
-    assert result.meta.cal_step.pixel_replace == "SKIPPED"
+    # Step is failed
+    assert result.meta.cal_step.pixel_replace == "FAILED"
 
     # Input is not modified
     assert result is not bad_model
@@ -373,8 +373,8 @@ def test_skip_unexpected_type_in_container():
     container = ModelContainer([bad_model])
     result = PixelReplaceStep.call(container)
 
-    # Step is skipped
-    assert result[0].meta.cal_step.pixel_replace == "SKIPPED"
+    # Step is failed
+    assert result[0].meta.cal_step.pixel_replace == "FAILED"
 
     # Input is not modified
     assert result[0] is not bad_model
diff --git a/jwst/regtest/test_niriss_image.py b/jwst/regtest/test_niriss_image.py
@@ -263,7 +263,7 @@ def test_niriss_tweakreg_no_sources(rtdata, fitsdiff_default_kwargs, log_watcher
     # Check the status of the step is set correctly in the files.
     mc = datamodels.ModelContainer(rtdata.input)
     for model in mc:
-        assert model.meta.cal_step.tweakreg != "SKIPPED"
+        assert model.meta.cal_step.tweakreg is None
 
     watcher = log_watcher(
         "jwst.tweakreg.tweakreg_catalog", message="No sources found in the image", level="warning"
@@ -272,7 +272,7 @@ def test_niriss_tweakreg_no_sources(rtdata, fitsdiff_default_kwargs, log_watcher
     watcher.assert_seen()
     with result:
         for model in result:
-            assert model.meta.cal_step.tweakreg == "SKIPPED"
+            assert model.meta.cal_step.tweakreg == "FAILED"
             result.shelve(model, modify=False)
 
 
diff --git a/jwst/stpipe/tests/test_utilities.py b/jwst/stpipe/tests/test_utilities.py
@@ -1,5 +1,6 @@
 """Test utility funcs"""
 
+import pytest
 from stdatamodels.jwst import datamodels
 from stpipe.utilities import resolve_step_class_alias
 
@@ -30,26 +31,28 @@ def test_resolve_step_class_alias():
         assert resolve_step_class_alias(full_class_name) == full_class_name
 
 
-def test_record_query_step_status():
+@pytest.mark.parametrize("fail_status", [None, "SKIPPED", "FAILED"])
+def test_record_query_step_status(fail_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
+    # modelcontainer case where all failed
     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"
+    record_step_status(model2, "test_step", success=False, status=fail_status)
+    expected = fail_status if fail_status is not None else "FAILED"
+    assert model2[0].meta.cal_step.instance["test_step"] == expected
+    assert model2[1].meta.cal_step.instance["test_step"] == expected
+    assert query_step_status(model2, "test_step") == expected
 
     # modelcontainer case with at least one complete
-    # currently the zeroth is checked, so this should return SKIPPED
+    # currently the zeroth is checked, so this should return FAILED
     model2.append(dm.MultiSpecModel())
     model2[2].meta.cal_step.instance["test_step"] = "COMPLETE"
-    assert query_step_status(model2, "test_step") == "SKIPPED"
+    assert query_step_status(model2, "test_step") == expected
 
     # test query not set
     model3 = dm.MultiSpecModel()
@@ -59,11 +62,17 @@ def test_record_query_step_status():
     model4 = dm.ModelLibrary([model])
     assert query_step_status(model4, "test_step") == "COMPLETE"
     model5 = dm.ModelLibrary(model2)
-    assert query_step_status(model5, "test_step") == "SKIPPED"
+    assert query_step_status(model5, "test_step") == expected
     model6 = dm.ModelLibrary([model3])
     assert query_step_status(model6, "test_step") == NOT_SET
 
 
+def test_record_status_invalid_value():
+    model = dm.MultiSpecModel()
+    with pytest.raises(ValueError, match="BAD_VALUE not in allowed values"):
+        record_step_status(model, "test_step", status="BAD_VALUE")
+
+
 def change_name_func(model):
     model.meta.filename = "changed"
     model.meta.cal_step.pixel_replace = "COMPLETE"
diff --git a/jwst/stpipe/utilities.py b/jwst/stpipe/utilities.py
@@ -25,9 +25,8 @@
     "SystemCall",
 ]
 
+ALLOWED_STEP_STATUS = ("COMPLETE", "SKIPPED", "FAILED")
 NOT_SET = "NOT SET"
-COMPLETE = "COMPLETE"
-SKIPPED = "SKIPPED"
 
 __all__ = [
     "all_steps",
@@ -134,7 +133,7 @@ def folder_traverse(folder_path, basename_regex=".+", path_exclude_regex="^$"):
                 yield os.path.join(root, file)  # noqa: PTH118
 
 
-def record_step_status(datamodel, cal_step, success=True):
+def record_step_status(datamodel, cal_step, success=True, status=None):
     """
     Record whether or not a step completed in ``meta.cal_step``.
 
@@ -144,17 +143,22 @@ def record_step_status(datamodel, cal_step, success=True):
                 `~jwst.datamodels.container.ModelContainer`, \
                 `~jwst.datamodels.library.ModelLibrary`
         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'
+    success : bool, optional
+        If True, set the status to "COMPLETE". If False, set "FAILED".
+        Ignored if ``status`` is not None.
+    status : {"COMPLETE", "SKIPPED", "FAILED"}, optional
+        The step status to record.
     """
-    if success:
-        status = COMPLETE
-    else:
-        status = SKIPPED
+    if status is None:
+        if success:
+            status = "COMPLETE"
+        else:
+            status = "FAILED"
+
+    if status not in ALLOWED_STEP_STATUS:
+        raise ValueError(f"Step status {status} not in allowed values: {ALLOWED_STEP_STATUS}")
 
     if isinstance(datamodel, Sequence):
         for model in datamodel:
@@ -167,8 +171,6 @@ def record_step_status(datamodel, cal_step, success=True):
     else:
         datamodel.meta.cal_step.instance[cal_step] = status
 
-    # TODO: standardize cal_step naming to point to the official step name
-
 
 def query_step_status(datamodel, cal_step):
     """
@@ -184,14 +186,14 @@ def query_step_status(datamodel, cal_step):
                 `~jwst.datamodels.container.ModelContainer`, \
                 `~jwst.datamodels.library.ModelLibrary`
         The datamodel or container of datamodels to check
-
     cal_step : str
         The attribute in meta.cal_step to check
 
     Returns
     -------
-    status : str
-        The status of the step in ``meta.cal_step``, typically 'COMPLETE' or 'SKIPPED'
+    status : {"COMPLETE", "SKIPPED", "FAILED"}
+        The status of the step in ``meta.cal_step``.  If not set, "NOT SET"
+        will be returned.
 
     Notes
     -----
diff --git a/jwst/tweakreg/tests/test_tweakreg.py b/jwst/tweakreg/tests/test_tweakreg.py
diff --git a/pyproject.toml b/pyproject.toml
