diff --git a/CHANGES.rst b/CHANGES.rst index c18dce5a16..97d5e9150e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,30 @@ 1.15.1 (unreleased) =================== -- +cube_build +---------- + +- Removed direct setting of the ``self.skip`` attribute from within the step + itself. [#8600] + +master_background +----------------- + +- Either of ``"background"`` or ``"bkg"`` in slit name now defines the slit + as a background slit, instead of ``"bkg"`` only. [#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) =================== diff --git a/jwst/cube_build/cube_build_step.py b/jwst/cube_build/cube_build_step.py index 15a9dde45d..7801169ba7 100755 --- a/jwst/cube_build/cube_build_step.py +++ b/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 @@ def process(self, input): 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 # ****************************************************************************** diff --git a/jwst/master_background/master_background_mos_step.py b/jwst/master_background/master_background_mos_step.py index 252155a3c4..45c641ff5c 100644 --- a/jwst/master_background/master_background_mos_step.py +++ b/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 from stdatamodels.jwst import datamodels @@ -108,7 +109,7 @@ def process(self, data): if not self.force_subtract and \ 'COMPLETE' in [data_model.meta.cal_step.back_sub, data_model.meta.cal_step.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 @@ def process(self, data): 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 @@ def process(self, data): # 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 diff --git a/jwst/master_background/master_background_step.py b/jwst/master_background/master_background_step.py index 76efbbff64..1ea83855ee 100755 --- a/jwst/master_background/master_background_step.py +++ b/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 @@ def process(self, input): # 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 @@ def process(self, input): "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,7 +160,7 @@ def process(self, input): "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 @@ -167,7 +168,7 @@ def process(self, input): 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 diff --git a/jwst/master_background/nirspec_utils.py b/jwst/master_background/nirspec_utils.py index d29c9da26b..153bf9f504 100644 --- a/jwst/master_background/nirspec_utils.py +++ b/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 diff --git a/jwst/master_background/tests/test_nirspec_corrections.py b/jwst/master_background/tests/test_nirspec_corrections.py index 5eaa41fcc8..9be59fd1e4 100644 --- a/jwst/master_background/tests/test_nirspec_corrections.py +++ b/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.""" diff --git a/jwst/pipeline/calwebb_spec2.py b/jwst/pipeline/calwebb_spec2.py index 9f3004f4a3..d75f44dc72 100644 --- a/jwst/pipeline/calwebb_spec2.py +++ b/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 diff --git a/jwst/pipeline/calwebb_spec3.py b/jwst/pipeline/calwebb_spec3.py index 9357903a17..23b9902a7c 100644 --- a/jwst/pipeline/calwebb_spec3.py +++ b/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 @@ def process(self, input): # 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() diff --git a/jwst/pixel_replace/pixel_replace_step.py b/jwst/pixel_replace/pixel_replace_step.py index 3efd7d2318..2c9b117e10 100644 --- a/jwst/pixel_replace/pixel_replace_step.py +++ b/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 @@ def process(self, input): 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 @@ def process(self, input): 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 diff --git a/jwst/stpipe/__init__.py b/jwst/stpipe/__init__.py index 637a36d0a6..8e30727f86 100644 --- a/jwst/stpipe/__init__.py +++ b/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'] diff --git a/jwst/stpipe/core.py b/jwst/stpipe/core.py index 01ca6921ad..2c86df37c8 100644 --- a/jwst/stpipe/core.py +++ b/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) diff --git a/jwst/stpipe/tests/test_utilities.py b/jwst/stpipe/tests/test_utilities.py index 0f35087181..0a1d543d48 100644 --- a/jwst/stpipe/tests/test_utilities.py +++ b/jwst/stpipe/tests/test_utilities.py @@ -1,9 +1,11 @@ """Test utility funcs""" 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 +27,30 @@ 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 + # currently the zeroth is checked, so this should return SKIPPED + model2.append(dm.MultiSpecModel()) + model2[2].meta.cal_step._instance['test_step'] = 'COMPLETE' + assert query_step_status(model2, 'test_step') == 'SKIPPED' + + # test query not set + model3 = dm.MultiSpecModel() + assert query_step_status(model3, 'test_step') == NOT_SET diff --git a/jwst/stpipe/utilities.py b/jwst/stpipe/utilities.py index f75af200b7..67a4769113 100644 --- a/jwst/stpipe/utilities.py +++ b/jwst/stpipe/utilities.py @@ -35,6 +35,7 @@ import logging import os import re +from collections.abc import Sequence # Configure logging logger = logging.getLogger(__name__) @@ -51,6 +52,9 @@ 'SystemCall', ] +NOT_SET = "NOT SET" +COMPLETE = "COMPLETE" +SKIPPED = "SKIPPED" def all_steps(): """List all classes subclassed from Step @@ -144,3 +148,60 @@ def folder_traverse(folder_path, basename_regex='.+', path_exclude_regex='^$'): for file in files: if basename_regex.match(file): yield os.path.join(root, file) + + +def record_step_status(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 + + 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 query_step_status(datamodel, cal_step): + """Query the status of a step in meta.cal_step + + Parameters + ---------- + datamodel : `~jwst.datamodels.JwstDataModel` or `~jwst.datamodels.ModelContainer` instance + 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' + + Notes + ----- + In principle, a step could set the COMPLETE status for only some subset + of models, so checking the zeroth model instance may not always be correct. + However, this is not currently done in the pipeline. This function should be + updated to accommodate that use-case as needed. + """ + if isinstance(datamodel, Sequence): + return getattr(datamodel[0].meta.cal_step, cal_step, NOT_SET) + else: + return getattr(datamodel.meta.cal_step, cal_step, NOT_SET) diff --git a/jwst/tweakreg/tweakreg_step.py b/jwst/tweakreg/tweakreg_step.py index d928cb166a..f0d7d68600 100644 --- a/jwst/tweakreg/tweakreg_step.py +++ b/jwst/tweakreg/tweakreg_step.py @@ -16,6 +16,7 @@ from tweakwcs.matchutils import XYXYMatch from jwst.datamodels import ModelContainer +from jwst.stpipe import record_step_status # LOCAL from ..stpipe import Step @@ -204,10 +205,8 @@ def process(self, input): self.log.warning("At least two exposures are required for image " "alignment.") self.log.warning("Nothing to do. Skipping 'TweakRegStep'...") - self.skip = True - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" - return input + record_step_status(images, "tweakreg", success=False) + return images # === start processing images === @@ -311,10 +310,8 @@ def process(self, input): self.log.warning("At least two exposures are required for " "image alignment.") self.log.warning("Nothing to do. Skipping 'TweakRegStep'...") - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" + record_step_status(images, "tweakreg", success=False) if not align_to_abs_refcat: - self.skip = True return images local_align_failed = True else: @@ -329,9 +326,7 @@ def process(self, input): "matched to a single reference source. Try to " "adjust 'tolerance' and/or 'separation' parameters.") self.log.warning("Skipping 'TweakRegStep'...") - self.skip = True - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" + record_step_status(images, "tweakreg", success=False) return images else: raise e @@ -341,9 +336,7 @@ def process(self, input): self.log.warning("Skipping relative alignment (stage 1)...") else: self.log.warning("Skipping 'TweakRegStep'...") - self.skip = True - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" + record_step_status(images, "tweakreg", success=False) return images if align_to_abs_refcat: @@ -461,9 +454,7 @@ def process(self, input): ) if local_align_failed or n_groups == 1: self.log.warning("Nothing to do. Skipping 'TweakRegStep'...") - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" - self.skip = True + record_step_status(images, "tweakreg", success=False) return images else: raise e @@ -482,9 +473,7 @@ def process(self, input): ) if local_align_failed or n_groups == 1: self.log.warning("Skipping 'TweakRegStep'...") - self.skip = True - for model in images: - model.meta.cal_step.tweakreg = "SKIPPED" + record_step_status(images, "tweakreg", success=False) return images else: raise e