From f4c281442fd02ebbf5822b8a2ec758bab1634560 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Wed, 11 Sep 2024 10:54:11 -0400 Subject: [PATCH 01/13] handle Sequence input for skip and save_results --- src/stpipe/step.py | 69 +++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index f956c454..7f0d508a 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -491,31 +491,34 @@ def run(self, *args): e, ) library.shelve(model, i) - elif isinstance(args[0], AbstractDataModel): - if self.class_alias is not None: - if isinstance(args[0], Sequence): - for model in args[0]: - try: - model[f"meta.cal_step.{self.class_alias}"] = ( - "SKIPPED" - ) - except AttributeError as e: # noqa: PERF203 - self.log.info( - "Could not record skip into DataModel " - "header: %s", - e, - ) - elif isinstance(args[0], AbstractDataModel): + + elif isinstance(args[0], Sequence) and self.class_alias is not None: + # handle ModelContainer or list of models + if isinstance(args[0][0], AbstractDataModel): + for model in args[0]: try: - args[0][ - f"meta.cal_step.{self.class_alias}" - ] = "SKIPPED" + setattr( + model.meta.cal_step, self.class_alias, "SKIPPED" + ) except AttributeError as e: self.log.info( - "Could not record skip into DataModel" - " header: %s", + "Could not record skip into DataModel " + "header: %s", e, ) + + elif isinstance(args[0], AbstractDataModel) and \ + self.class_alias is not None: + try: + args[0][ + f"meta.cal_step.{self.class_alias}" + ] = "SKIPPED" + except AttributeError as e: + self.log.info( + "Could not record skip into DataModel" + " header: %s", + e, + ) step_result = args[0] else: if self.prefetch_references: @@ -558,7 +561,7 @@ def run(self, *args): # Save the output file if one was specified if not self.skip and self.save_results: # Setup the save list. - if not isinstance(step_result, list | tuple): + if not isinstance(step_result, Sequence): results_to_save = [step_result] else: results_to_save = step_result @@ -1008,17 +1011,21 @@ def save_model( # leaving modify=True in case saving modify the file model.shelve(m, i) return output_paths + elif isinstance(model, Sequence): - save_model_func = partial( - self.save_model, - suffix=suffix, - force=force, - **components, - ) - output_path = model.save( - path=output_file, - save_model_func=save_model_func, - ) + output_paths = [] + for i, m in enumerate(model): + output_paths.append( + self.save_model( + m, + idx=i, + suffix=suffix, + force=force, + **components, + ) + ) + return output_paths + else: # Search for an output file name. if self.output_use_model or ( From 7afa00cd2b70bad80250b7e72fe341844ddbe68a Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Mon, 23 Sep 2024 17:55:41 -0400 Subject: [PATCH 02/13] make Sequence calls work with SourceModelContainer --- src/stpipe/step.py | 74 +++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 40 deletions(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 7f0d508a..80cd962c 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -560,34 +560,7 @@ def run(self, *args): # Save the output file if one was specified if not self.skip and self.save_results: - # Setup the save list. - if not isinstance(step_result, Sequence): - results_to_save = [step_result] - else: - results_to_save = step_result - - for idx, result in enumerate(results_to_save): - if len(results_to_save) <= 1: - idx = None - if isinstance( - result, (AbstractDataModel | AbstractModelLibrary) - ): - self.save_model(result, idx=idx) - elif hasattr(result, "save"): - try: - output_path = self.make_output_path(idx=idx) - except AttributeError: - self.log.warning( - "`save_results` has been requested, but cannot" - " determine filename." - ) - self.log.warning( - "Specify an output file with `--output_file` or set" - " `--save_results=false`" - ) - else: - self.log.info("Saving file %s", output_path) - result.save(output_path, overwrite=True) + self.save_model(step_result) if not self.skip: self.log.info("Step %s done", self.name) @@ -995,6 +968,9 @@ def save_model( if not force and not self.save_results and not output_file: return None + if model is None: + return None + if isinstance(model, AbstractModelLibrary): output_paths = [] with model: @@ -1013,20 +989,36 @@ def save_model( return output_paths elif isinstance(model, Sequence): - output_paths = [] - for i, m in enumerate(model): - output_paths.append( - self.save_model( - m, - idx=i, - suffix=suffix, - force=force, - **components, + if not hasattr(model, "save"): + # list of datamodels, e.g. ModelContainer + output_paths = [] + for i, m in enumerate(model): + idx = None if len(model) == 1 else i + output_paths.append( + self.save_model( + m, + idx=idx, + suffix=suffix, + force=force, + **components, + ) ) + return output_paths + else: + # JWST SourceModelContainer takes this path + save_model_func = partial( + self.save_model, + suffix=suffix, + force=force, + **components, ) - return output_paths + output_path = model.save( + path=output_file, + save_model_func=save_model_func, + ) + return output_path - else: + elif hasattr(model, "save"): # Search for an output file name. if self.output_use_model or ( output_file is None and not self.search_output_file @@ -1042,8 +1034,10 @@ def save_model( ) ) self.log.info("Saved model in %s", output_path) + return output_path - return output_path + else: + return @property def make_output_path(self): From 4b63ec229a6ac39dde69a290d241689a6693827f Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Wed, 25 Sep 2024 15:22:09 -0400 Subject: [PATCH 03/13] fix stpipe unit tests --- src/stpipe/step.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 80cd962c..1b725e63 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -988,7 +988,7 @@ def save_model( model.shelve(m, i) return output_paths - elif isinstance(model, Sequence): + elif isinstance(model, Sequence) and not isinstance(model, str): if not hasattr(model, "save"): # list of datamodels, e.g. ModelContainer output_paths = [] From 8298c38b0e9e6997e8bf8260aee49d67e2d496e2 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Mon, 7 Oct 2024 15:19:26 -0400 Subject: [PATCH 04/13] added changelog entry --- changes/190.misc.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/190.misc.rst diff --git a/changes/190.misc.rst b/changes/190.misc.rst new file mode 100644 index 00000000..5cfbcf04 --- /dev/null +++ b/changes/190.misc.rst @@ -0,0 +1 @@ +improve support for list-like input into Step From 9eeddb145336d33cf2aa2571233dc4f3a59941f3 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 8 Oct 2024 09:36:30 -0400 Subject: [PATCH 05/13] fix overwriting same file multiple times when list of list passed in --- src/stpipe/step.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 1b725e63..778eeb7d 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -993,16 +993,18 @@ def save_model( # list of datamodels, e.g. ModelContainer output_paths = [] for i, m in enumerate(model): - idx = None if len(model) == 1 else i - output_paths.append( - self.save_model( - m, - idx=idx, - suffix=suffix, - force=force, - **components, + # ignore list of lists. individual steps should handle this + if not isinstance(m, Sequence): + idx = None if len(model) == 1 else i + output_paths.append( + self.save_model( + m, + idx=idx, + suffix=suffix, + force=force, + **components, + ) ) - ) return output_paths else: # JWST SourceModelContainer takes this path From 30888d4565905c09e1d153077d5b44cf725c36ed Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 8 Oct 2024 11:09:58 -0400 Subject: [PATCH 06/13] added simple tests of step save methods --- src/stpipe/step.py | 2 +- tests/test_step.py | 123 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 778eeb7d..683cf55b 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -990,7 +990,7 @@ def save_model( elif isinstance(model, Sequence) and not isinstance(model, str): if not hasattr(model, "save"): - # list of datamodels, e.g. ModelContainer + # list of datamodels, e.g. JWST ModelContainer output_paths = [] for i, m in enumerate(model): # ignore list of lists. individual steps should handle this diff --git a/tests/test_step.py b/tests/test_step.py index 972f84a9..ea38a0b4 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -1,7 +1,9 @@ """Test step.Step""" +import copy import logging import re +from collections.abc import Sequence from typing import ClassVar import asdf @@ -9,6 +11,7 @@ import stpipe.config_parser as cp from stpipe import cmdline +from stpipe.datamodel import AbstractDataModel from stpipe.pipeline import Pipeline from stpipe.step import Step @@ -411,3 +414,123 @@ def test_log_records(): pipeline.run() assert any(r == "This step has called out a warning." for r in pipeline.log_records) + + +class StepWithModel(Step): + """A step that immediately saves the model it gets passed in""" + + spec = """ + output_ext = string(default='simplestep') + save_results = boolean(default=True) + """ + # spec = """ + # + # skip = bool(default=False) + # """ + + def process(self, input_model): + return input_model + + +class SimpleDataModel(AbstractDataModel): + """A simple data model""" + + @property + def crds_observatory(self): + return "jwst" + + # @property + # def meta(self): + # return {"filename": "test.fits"} + + def get_crds_parameters(self): + return {"test": "none"} + + def save(self, path, dir_path=None, *args, **kwargs): + saveid = getattr(self, "saveid", None) + if saveid is not None: + print(f"here {saveid}") + fname = saveid+"-saved.txt" + with open(fname, "w") as f: + f.write(f"{path}") + return fname + return None + + +def test_save(tmp_cwd): + + model = SimpleDataModel() + model.saveid = "test" + step = StepWithModel() + step.run(model) + assert (tmp_cwd / "test-saved.txt").exists() + + +@pytest.fixture(scope="function") +def model_list(): + model = SimpleDataModel() + model_list = [copy.deepcopy(model) for _ in range(3)] + for i, model in enumerate(model_list): + model.saveid = f"test{i}" + return model_list + + +def test_save_list(tmp_cwd, model_list): + step = StepWithModel() + step.run(model_list) + for i in range(3): + assert (tmp_cwd / f"test{i}-saved.txt").exists() + + +class SimpleContainer(Sequence): + + def __init__(self, models): + self._models = models + + def __len__(self): + return len(self._models) + + def __getitem__(self, idx): + return self._models[idx] + + def __iter__(self): + yield from self._models + + def insert(self, index, model): + self._models.insert(index, model) + + def append(self, model): + self._models.append(model) + + def extend(self, model): + self._models.extend(model) + + def pop(self, index=-1): + self._models.pop(index) + + +def test_save_container(tmp_cwd, model_list): + """ensure list-like save still works for non-list sequence""" + container = SimpleContainer(model_list) + step = StepWithModel() + step.run(container) + for i in range(3): + assert (tmp_cwd / f"test{i}-saved.txt").exists() + + +def test_save_tuple_with_nested_list(tmp_cwd, model_list): + """ + in rare cases, multiple outputs are returned from step as tuple. + One example is the jwst badpix_selfcal step, which returns one sci exposure + and a list containing an arbitrary number of background exposures. + Expected behavior in this case, at least at time of writing, is to save the + science exposure and ignore the list + """ + single_model = SimpleDataModel() + single_model.saveid = "test" + container = (single_model, model_list) + step = StepWithModel() + step.run(container) + assert (tmp_cwd / "test-saved.txt").exists() + for i in range(3): + assert not (tmp_cwd / f"test{i}-saved.txt").exists() From 83bbea08aca17b7017593375a2cafcfe8961dc9f Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 8 Oct 2024 11:48:18 -0400 Subject: [PATCH 07/13] added unit test for ModelContainer with custom save method --- tests/test_step.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_step.py b/tests/test_step.py index ea38a0b4..dade4b5e 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -509,6 +509,15 @@ def pop(self, index=-1): self._models.pop(index) +class SimpleContainerWithSave(SimpleContainer): + + def save(self, path, dir_path=None, *args, **kwargs): + for model in self._models[1:]: + # skip the first model to test that the save method is called + # rather than just looping over all models like in the without-save case + model.save(path, dir_path, *args, **kwargs) + + def test_save_container(tmp_cwd, model_list): """ensure list-like save still works for non-list sequence""" container = SimpleContainer(model_list) @@ -518,6 +527,16 @@ def test_save_container(tmp_cwd, model_list): assert (tmp_cwd / f"test{i}-saved.txt").exists() +def test_save_container_with_save_method(tmp_cwd, model_list): + """ensure list-like save still works for non-list sequence""" + container = SimpleContainerWithSave(model_list) + step = StepWithModel() + step.run(container) + assert not (tmp_cwd / "test0-saved.txt").exists() + assert (tmp_cwd / "test1-saved.txt").exists() + assert (tmp_cwd / "test2-saved.txt").exists() + + def test_save_tuple_with_nested_list(tmp_cwd, model_list): """ in rare cases, multiple outputs are returned from step as tuple. From 622b58a67910f3d26c35313ad024d67cbc3af06c Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 8 Oct 2024 13:39:36 -0400 Subject: [PATCH 08/13] added simple tests of skip --- tests/test_step.py | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/tests/test_step.py b/tests/test_step.py index dade4b5e..9693698b 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -423,12 +423,11 @@ class StepWithModel(Step): output_ext = string(default='simplestep') save_results = boolean(default=True) """ - # spec = """ - # - # skip = bool(default=False) - # """ def process(self, input_model): + # make a change to ensure step skip is working + # without having to define SimpleDataModel.meta.stepname + input_model.stepstatus = "COMPLETED" return input_model @@ -439,17 +438,12 @@ class SimpleDataModel(AbstractDataModel): def crds_observatory(self): return "jwst" - # @property - # def meta(self): - # return {"filename": "test.fits"} - def get_crds_parameters(self): return {"test": "none"} def save(self, path, dir_path=None, *args, **kwargs): saveid = getattr(self, "saveid", None) if saveid is not None: - print(f"here {saveid}") fname = saveid+"-saved.txt" with open(fname, "w") as f: f.write(f"{path}") @@ -466,6 +460,15 @@ def test_save(tmp_cwd): assert (tmp_cwd / "test-saved.txt").exists() +def test_skip(): + model = SimpleDataModel() + step = StepWithModel() + step.skip = True + out = step.run(model) + assert not hasattr(out, "stepstatus") + assert out is model + + @pytest.fixture(scope="function") def model_list(): model = SimpleDataModel() @@ -527,6 +530,16 @@ def test_save_container(tmp_cwd, model_list): assert (tmp_cwd / f"test{i}-saved.txt").exists() +def test_skip_container(tmp_cwd, model_list): + step = StepWithModel() + step.skip = True + out = step.run(model_list) + assert not hasattr(out, "stepstatus") + for i, model in enumerate(out): + assert not hasattr(model, "stepstatus") + assert model_list[i] is model + + def test_save_container_with_save_method(tmp_cwd, model_list): """ensure list-like save still works for non-list sequence""" container = SimpleContainerWithSave(model_list) From 843c18cd6c74f6c294d239e55ad3195094e40529 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Tue, 8 Oct 2024 13:46:44 -0400 Subject: [PATCH 09/13] fix silly problem with unit test --- tests/test_step.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_step.py b/tests/test_step.py index 9693698b..c8e8b79f 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -427,7 +427,11 @@ class StepWithModel(Step): def process(self, input_model): # make a change to ensure step skip is working # without having to define SimpleDataModel.meta.stepname - input_model.stepstatus = "COMPLETED" + if isinstance(input_model, SimpleDataModel): + input_model.stepstatus = "COMPLETED" + elif isinstance(input_model, SimpleContainer): + for model in input_model: + model.stepstatus = "COMPLETED" return input_model From 15936e93f78eb99a1fd380fd085ac084f8c2d6bb Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Wed, 9 Oct 2024 17:14:22 -0400 Subject: [PATCH 10/13] less invasive changes to save logic in step --- src/stpipe/step.py | 90 +++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 41 deletions(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 683cf55b..6d641e2b 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -492,9 +492,11 @@ def run(self, *args): ) library.shelve(model, i) - elif isinstance(args[0], Sequence) and self.class_alias is not None: + elif (isinstance(args[0], Sequence)) and \ + (not isinstance(args[0], str)) and \ + (self.class_alias is not None): # handle ModelContainer or list of models - if isinstance(args[0][0], AbstractDataModel): + if args[0] and isinstance(args[0][0], AbstractDataModel): for model in args[0]: try: setattr( @@ -515,8 +517,7 @@ def run(self, *args): ] = "SKIPPED" except AttributeError as e: self.log.info( - "Could not record skip into DataModel" - " header: %s", + "Could not record skip into DataModel header: %s", e, ) step_result = args[0] @@ -560,7 +561,37 @@ def run(self, *args): # Save the output file if one was specified if not self.skip and self.save_results: - self.save_model(step_result) + # Setup the save list. + if isinstance(step_result, Sequence): + if hasattr(step_result, "save") or isinstance(step_result, str): + results_to_save = [step_result] + else: + results_to_save = step_result + else: + results_to_save = [step_result] + + for idx, result in enumerate(results_to_save): + if len(results_to_save) <= 1: + idx = None + if isinstance( + result, (AbstractDataModel | AbstractModelLibrary) + ): + self.save_model(result, idx=idx) + elif hasattr(result, "save"): + try: + output_path = self.make_output_path(idx=idx) + except AttributeError: + self.log.warning( + "`save_results` has been requested, but cannot" + " determine filename." + ) + self.log.warning( + "Specify an output file with `--output_file` or set" + " `--save_results=false`" + ) + else: + self.log.info("Saving file %s", output_path) + result.save(output_path, overwrite=True) if not self.skip: self.log.info("Step %s done", self.name) @@ -988,39 +1019,18 @@ def save_model( model.shelve(m, i) return output_paths - elif isinstance(model, Sequence) and not isinstance(model, str): - if not hasattr(model, "save"): - # list of datamodels, e.g. JWST ModelContainer - output_paths = [] - for i, m in enumerate(model): - # ignore list of lists. individual steps should handle this - if not isinstance(m, Sequence): - idx = None if len(model) == 1 else i - output_paths.append( - self.save_model( - m, - idx=idx, - suffix=suffix, - force=force, - **components, - ) - ) - return output_paths - else: - # JWST SourceModelContainer takes this path - save_model_func = partial( - self.save_model, - suffix=suffix, - force=force, - **components, - ) - output_path = model.save( - path=output_file, - save_model_func=save_model_func, - ) - return output_path - - elif hasattr(model, "save"): + elif isinstance(model, Sequence): + save_model_func = partial( + self.save_model, + suffix=suffix, + force=force, + **components, + ) + output_path = model.save( + path=output_file, + save_model_func=save_model_func, + ) + else: # Search for an output file name. if self.output_use_model or ( output_file is None and not self.search_output_file @@ -1036,10 +1046,8 @@ def save_model( ) ) self.log.info("Saved model in %s", output_path) - return output_path - else: - return + return output_path @property def make_output_path(self): From 956dc043c282bc052769f63dee0962d94310d59b Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Thu, 10 Oct 2024 15:45:57 -0400 Subject: [PATCH 11/13] revert step.py to main --- src/stpipe/step.py | 57 +++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 6d641e2b..f956c454 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -491,35 +491,31 @@ def run(self, *args): e, ) library.shelve(model, i) - - elif (isinstance(args[0], Sequence)) and \ - (not isinstance(args[0], str)) and \ - (self.class_alias is not None): - # handle ModelContainer or list of models - if args[0] and isinstance(args[0][0], AbstractDataModel): - for model in args[0]: + elif isinstance(args[0], AbstractDataModel): + if self.class_alias is not None: + if isinstance(args[0], Sequence): + for model in args[0]: + try: + model[f"meta.cal_step.{self.class_alias}"] = ( + "SKIPPED" + ) + except AttributeError as e: # noqa: PERF203 + self.log.info( + "Could not record skip into DataModel " + "header: %s", + e, + ) + elif isinstance(args[0], AbstractDataModel): try: - setattr( - model.meta.cal_step, self.class_alias, "SKIPPED" - ) + args[0][ + f"meta.cal_step.{self.class_alias}" + ] = "SKIPPED" except AttributeError as e: self.log.info( - "Could not record skip into DataModel " - "header: %s", + "Could not record skip into DataModel" + " header: %s", e, ) - - elif isinstance(args[0], AbstractDataModel) and \ - self.class_alias is not None: - try: - args[0][ - f"meta.cal_step.{self.class_alias}" - ] = "SKIPPED" - except AttributeError as e: - self.log.info( - "Could not record skip into DataModel header: %s", - e, - ) step_result = args[0] else: if self.prefetch_references: @@ -562,13 +558,10 @@ def run(self, *args): # Save the output file if one was specified if not self.skip and self.save_results: # Setup the save list. - if isinstance(step_result, Sequence): - if hasattr(step_result, "save") or isinstance(step_result, str): - results_to_save = [step_result] - else: - results_to_save = step_result - else: + if not isinstance(step_result, list | tuple): results_to_save = [step_result] + else: + results_to_save = step_result for idx, result in enumerate(results_to_save): if len(results_to_save) <= 1: @@ -999,9 +992,6 @@ def save_model( if not force and not self.save_results and not output_file: return None - if model is None: - return None - if isinstance(model, AbstractModelLibrary): output_paths = [] with model: @@ -1018,7 +1008,6 @@ def save_model( # leaving modify=True in case saving modify the file model.shelve(m, i) return output_paths - elif isinstance(model, Sequence): save_model_func = partial( self.save_model, From f9be7e7395feb0797742ca80d14ce39b17e3e3d4 Mon Sep 17 00:00:00 2001 From: Ned Molter Date: Thu, 10 Oct 2024 15:51:08 -0400 Subject: [PATCH 12/13] marked unit test of non-save container with xfail --- changes/190.misc.rst | 1 - tests/test_step.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 changes/190.misc.rst diff --git a/changes/190.misc.rst b/changes/190.misc.rst deleted file mode 100644 index 5cfbcf04..00000000 --- a/changes/190.misc.rst +++ /dev/null @@ -1 +0,0 @@ -improve support for list-like input into Step diff --git a/tests/test_step.py b/tests/test_step.py index c8e8b79f..c31d236f 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -525,6 +525,7 @@ def save(self, path, dir_path=None, *args, **kwargs): model.save(path, dir_path, *args, **kwargs) +@pytest.mark.xfail(reason="Looping over models only works for list and tuple. This should be fixed.") def test_save_container(tmp_cwd, model_list): """ensure list-like save still works for non-list sequence""" container = SimpleContainer(model_list) @@ -545,7 +546,7 @@ def test_skip_container(tmp_cwd, model_list): def test_save_container_with_save_method(tmp_cwd, model_list): - """ensure list-like save still works for non-list sequence""" + """ensure top-level save method is called for sequence""" container = SimpleContainerWithSave(model_list) step = StepWithModel() step.run(container) From f6e2a84f2b8787bf92c53608be6b6491b172f304 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 10 Oct 2024 19:58:48 +0000 Subject: [PATCH 13/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_step.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_step.py b/tests/test_step.py index c31d236f..ea22e9be 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -448,7 +448,7 @@ def get_crds_parameters(self): def save(self, path, dir_path=None, *args, **kwargs): saveid = getattr(self, "saveid", None) if saveid is not None: - fname = saveid+"-saved.txt" + fname = saveid + "-saved.txt" with open(fname, "w") as f: f.write(f"{path}") return fname @@ -525,7 +525,9 @@ def save(self, path, dir_path=None, *args, **kwargs): model.save(path, dir_path, *args, **kwargs) -@pytest.mark.xfail(reason="Looping over models only works for list and tuple. This should be fixed.") +@pytest.mark.xfail( + reason="Looping over models only works for list and tuple. This should be fixed." +) def test_save_container(tmp_cwd, model_list): """ensure list-like save still works for non-list sequence""" container = SimpleContainer(model_list)