Skip to content

Commit

Permalink
Treat parameters explicitly passed on instantiation as initialized. A…
Browse files Browse the repository at this point in the history
…llow explicit CRDS disable
  • Loading branch information
melanieclarke committed Oct 10, 2024
1 parent bb2fdb5 commit 640271b
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 10 deletions.
10 changes: 9 additions & 1 deletion src/stpipe/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,15 @@ 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:
new_step = val.from_config_section(
cfg,
parent=self,
name=key,
config_file=self.config_file,
config_file=self.config_file
)
else:
new_step = val(
Expand All @@ -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
Expand Down
25 changes: 18 additions & 7 deletions src/stpipe/step.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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`
Expand All @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand All @@ -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()
Expand Down
66 changes: 64 additions & 2 deletions tests/test_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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

Expand All @@ -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

0 comments on commit 640271b

Please sign in to comment.