Set FAILED status for steps that did not succeed

melanieclarke · melanieclarke · commit b3895dfbf459 · 2026-04-29T14:54:56.000-04:00
diff --git a/changes/10497.stpipe.rst b/changes/10497.stpipe.rst
@@ -0,0 +1 @@
+Allow the ``cal_step`` status keyword to be set to FAILED (instead of SKIPPED) for steps that were attempted but did not succeed.
diff --git a/docs/jwst/references_general/references_general.rst b/docs/jwst/references_general/references_general.rst
@@ -481,8 +481,8 @@ As each pipeline step is applied to a science data product, it will record a sta
 header keyword of the science data product. The current list of step status keyword names is given
 in the following table. These status keywords may be included in the primary header of reference
 files, in order to maintain a history of the data that went into creating the reference file.
-Allowed values for the status keywords are 'COMPLETE' and 'SKIPPED'. Absence of a particular keyword
-is understood to mean that step was not even attempted.
+Allowed values for the status keywords are 'COMPLETE', 'SKIPPED', or 'FAILED'. Absence of a particular
+keyword is understood to mean that step was not even attempted.
 
 Table 1.  Keywords Documenting Which Pipeline Steps Have Been Performed.
 
@@ -540,6 +540,7 @@ S_STRAY     Straylight correction
 S_SUPERB    Superbias subtraction
 S_TACNTR    Source position from TA verification image
 S_TELEMI    Telescope emission correction
+S_TRCMOD    Adaptive trace modeling
 S_TSPHOT    TSO imaging photometry
 S_TWKREG    Tweakreg image alignment
 S_WAVCOR    Wavelength correction
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
diff --git a/jwst/tweakreg/tests/test_tweakreg.py b/jwst/tweakreg/tests/test_tweakreg.py
diff --git a/pyproject.toml b/pyproject.toml

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Allow the ``cal_step`` status keyword to be set to FAILED (instead of SKIPPED) for steps that were attempted but did not succeed.