diff --git a/src/stpipe/pipeline.py b/src/stpipe/pipeline.py index 70eaf00c..356b46bd 100644 --- a/src/stpipe/pipeline.py +++ b/src/stpipe/pipeline.py @@ -37,6 +37,7 @@ def __init__(self, *args, **kwargs): Step.__init__(self, *args, **kwargs) # Configure all of the steps + step_parameters = kwargs.get('steps', {}) for key, val in self.step_defs.items(): cfg = self.steps.get(key) if cfg is not None: @@ -44,7 +45,7 @@ def __init__(self, *args, **kwargs): cfg, parent=self, name=key, - config_file=self.config_file, + config_file=self.config_file ) else: new_step = val( @@ -54,6 +55,13 @@ def __init__(self, *args, **kwargs): **kwargs.get(key, {}), ) + # Make sure explicitly passed parameters for sub-steps + # are marked as initialized + input_parameters = step_parameters.get(key, {}) + for param in input_parameters: + if param in new_step._initialized: + new_step._initialized[param] = True + setattr(self, key, new_step) @property diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 6a697ffd..b6ef2fa1 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -364,8 +364,10 @@ def __init__( for key, val in kws.items(): setattr(self, key, val) - # Mark them as uninitialized, from the user standpoint - self._initialized[key] = False + # Mark them as uninitialized, from the user standpoint, + # unless they were explicitly passed on instantiation + if key not in self._keywords or parent is not None: + self._initialized[key] = False # Create a new logger for this step self.log = log.getLogger(self.qualified_name) @@ -439,8 +441,10 @@ def run(self, *args, **kwargs): The order of parameter checking and overrides is: 1. spec default value for the step 2. keyword parameters or configuration set on step initialization - 3. CRDS parameters if available - 4. step attributes directly set by the user before calling run + 3. CRDS parameters if available and if CRDS checks are not disabled + 4. step attributes explicitly set by the user before calling run, + either on instantiation or by directly setting the attribute + after instantiation. 5. keyword parameters passed directly to the run call Only 1 and 2 are checked if the step was created via `call` @@ -460,6 +464,9 @@ def run(self, *args, **kwargs): self.log.info("Step %s running with args %s.", self.name, args) + # Check for explicit disable for CRDS parameters + disable_crds_steppars = kwargs.pop('disable_crds_steppars', None) + # Get parameters from user parameters = None if kwargs: @@ -478,7 +485,8 @@ def run(self, *args, **kwargs): # Build config from CRDS + user keywords try: - parameters, _ = self.build_config(filename, **kwargs) + parameters, _ = self.build_config( + filename, disable=disable_crds_steppars, **kwargs) except (NotImplementedError, FileNotFoundError): # Catch steps that cannot build a config # (e.g. post hooks created from local functions, @@ -1381,7 +1389,7 @@ def update_pars(self, parameters, skip_initialized=False): ) @classmethod - def build_config(cls, input, **kwargs): # noqa: A002 + def build_config(cls, input, disable=None, **kwargs): # noqa: A002 """Build the ConfigObj to initialize a Step A Step config is built in the following order: @@ -1395,6 +1403,9 @@ def build_config(cls, input, **kwargs): # noqa: A002 input : str or None Input file + disable : bool, optional + Do not retrieve parameters from CRDS. If None, check global settings. + kwargs : dict Keyword arguments that specify Step parameters. @@ -1406,7 +1417,7 @@ def build_config(cls, input, **kwargs): # noqa: A002 logger_name = cls.__name__ log_cls = log.getLogger(logger_name) if input: - config = cls.get_config_from_reference(input) + config = cls.get_config_from_reference(input, disable=disable) else: log_cls.info("No filename given, cannot retrieve config from CRDS") config = config_parser.ConfigObj() diff --git a/tests/test_step.py b/tests/test_step.py index 4cbe8245..3b541b02 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -462,6 +462,38 @@ def test_step_run_crds_values(step_class): assert step._initialized["str1"] is True +@pytest.mark.usefixtures("_mock_crds_reffile") +@pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep]) +def test_step_run_disable_crds_explicit(step_class): + """Test that CRDS parameters are not checked if disabled.""" + step = step_class() + step.process = lambda *args: None + + assert step.str1 == "default" + assert step._initialized["str1"] is False + + step.run("science.fits", disable_crds_steppars=True) + assert step.str1 == "default" + assert step._initialized["str1"] is False + + +@pytest.mark.usefixtures("_mock_crds_reffile") +@pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep]) +def test_step_run_disable_crds_via_environment(monkeypatch, step_class): + """Test that CRDS parameters are not checked if disabled.""" + step = step_class() + step.process = lambda *args: None + + assert step.str1 == "default" + assert step._initialized["str1"] is False + + monkeypatch.setenv('STPIPE_DISABLE_CRDS_STEPPARS', 'True') + + step.run("science.fits") + assert step.str1 == "default" + assert step._initialized["str1"] is False + + @pytest.mark.usefixtures("_mock_crds_reffile") @pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep]) def test_step_run_initialized_values(step_class): @@ -480,6 +512,21 @@ def test_step_run_initialized_values(step_class): assert step._initialized["str1"] is True +@pytest.mark.usefixtures("_mock_crds_reffile") +@pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep]) +def test_step_run_initialized_values_on_instantiation(step_class): + """Test that parameters pre-set are not overridden when run is called.""" + step = step_class(str1='on instantiation') + step.process = lambda *args: None + + assert step.str1 == "on instantiation" + assert step._initialized["str1"] is True + + step.run("science.fits") + assert step.str1 == "on instantiation" + assert step._initialized["str1"] is True + + @pytest.mark.usefixtures("_mock_crds_reffile") @pytest.mark.parametrize("step_class", [SimplePipe, SimpleStep]) def test_step_run_keyword_values(step_class): @@ -576,8 +623,8 @@ def test_pipe_run_step_values_skip_initialized(): pipe = SimplePipe() pipe.process = lambda *args: None - # Step parameters are initialized when created via the pipeline - # from defaults + # Step parameters are not initialized when created via the + # pipeline from defaults assert pipe.step1.str1 == "default" assert pipe.step1._initialized["str1"] is False @@ -588,3 +635,18 @@ def test_pipe_run_step_values_skip_initialized(): pipe.run("science.fits") assert pipe.step1.str1 == "from user" assert pipe.step1._initialized["str1"] is True + + +@pytest.mark.usefixtures("_mock_crds_reffile") +def test_pipe_run_step_values_skip_initialized(): + """Test that initialized parameters are not overridden.""" + pipe = SimplePipe(steps={'step1': {'str1': 'on instantiation'}}) + pipe.process = lambda *args: None + + assert pipe.step1.str1 == "on instantiation" + assert pipe.step1._initialized["str1"] is True + + # Parameters are not overridden by CRDS + pipe.run("science.fits") + assert pipe.step1.str1 == "on instantiation" + assert pipe.step1._initialized["str1"] is True