diff --git a/octue/resources/analysis.py b/octue/resources/analysis.py index e3f94103e..cafa9f756 100644 --- a/octue/resources/analysis.py +++ b/octue/resources/analysis.py @@ -2,7 +2,6 @@ import logging from octue.definitions import OUTPUT_STRANDS -from octue.exceptions import ProtectedAttributeException from octue.mixins import Identifiable, Loggable, Serialisable, Taggable from octue.resources.manifest import Manifest from octue.utils.encoders import OctueJSONEncoder @@ -62,27 +61,11 @@ def __init__(self, twine, skip_checks=False, **kwargs): # Pop any possible strand data sources before init superclasses (and tie them to protected attributes) strand_kwargs = ((name, kwargs.pop(name, None)) for name in ALL_STRANDS) for strand_name, strand_data in strand_kwargs: - self.__setattr__(f"_{strand_name}", strand_data) + setattr(self, f"{strand_name}", strand_data) # Init superclasses super().__init__(**kwargs) - def __setattr__(self, name, value): - """ Override setters for protected attributes (the strand contents may change, but the strands themselves - shouldn't be changed after instantiation) - """ - if name in ALL_STRANDS: - raise ProtectedAttributeException(f"You cannot set {name} on an instantiated Analysis") - - super().__setattr__(name, value) - - def __getattr__(self, name): - """ Override public getters to point to protected attributes (the strand contents may change, but the strands - themselves shouldn't be changed after instantiation) - """ - if name in ALL_STRANDS: - return getattr(self, f"_{name}", None) - def finalise(self, output_dir=None): """ Validates and serialises output_values and output_manifest, optionally writing them to files diff --git a/octue/runner.py b/octue/runner.py index 2c226cbfd..1565cdbff 100644 --- a/octue/runner.py +++ b/octue/runner.py @@ -109,8 +109,8 @@ def _update_manifest_path(manifest, pathname): if pathname.endswith(".json"): manifest.path = os.path.split(pathname)[0] - # Otherwise do nothing and rely on manifest having its path variable set already - return manifest + # Otherwise do nothing and rely on manifest having its path variable set already + return manifest def run( self, diff --git a/octue/templates/template-using-manifests/app.py b/octue/templates/template-using-manifests/app.py index a294c5c45..0fc6107df 100644 --- a/octue/templates/template-using-manifests/app.py +++ b/octue/templates/template-using-manifests/app.py @@ -20,7 +20,7 @@ def run(analysis, *args, **kwargs): """ # You can use the attached logger to record debug statements, general information, warnings or errors - analysis.logger.info(f"Starting clean up of files in {analysis.input_dir}") + analysis.logger.info(f"Starting clean up of files in {analysis.input_manifest.absolute_path}") # Get the configuration value for our time averaging window (or if not present, use the default specified in # the twine) diff --git a/setup.py b/setup.py index c7442d468..f5b0c5eec 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ setup( name="octue", - version="0.1.5", + version="0.1.6", py_modules=["cli"], install_requires=["click>=7.1.2", "twined==0.0.13"], # Dev note: you also need to bump twined in tox.ini url="https://www.github.com/octue/octue-sdk-python", diff --git a/tests/resources/test_analysis.py b/tests/resources/test_analysis.py index f2b8aaccc..0c842b545 100644 --- a/tests/resources/test_analysis.py +++ b/tests/resources/test_analysis.py @@ -1,6 +1,3 @@ -import os - -from octue import exceptions from octue.resources import Analysis from twined import Twine from ..base import BaseTestCase @@ -20,23 +17,11 @@ def test_instantiate_analysis_with_twine(self): analysis = Analysis(twine=Twine(source="{}")) self.assertEqual(analysis.__class__.__name__, "Analysis") - def test_protected_setter(self): - """ Ensures that protected attributes can't be set + def test_non_existent_attributes_cannot_be_retrieved(self): + """ Ensure attributes that don't exist on Analysis aren't retrieved as None and instead raise an error. See + https://github.com/octue/octue-sdk-python/issues/45 for reasoning behind adding this. """ - analysis = Analysis(twine="{}") - with self.assertRaises(exceptions.ProtectedAttributeException) as error: - analysis.configuration_values = {} - - self.assertIn("You cannot set configuration_values on an instantiated Analysis", error.exception.args[0]) + analysis = Analysis(twine=Twine(source="{}")) - def test_protected_getter(self): - """ Ensures that protected attributes can't be set - """ - analysis = Analysis( - twine=str(os.path.join(self.data_path, "twines", "valid_schema_twine.json")), - configuration_values={"n_iterations": 5}, - input_values={"height": 5}, - output_values={}, - ) - cfg = analysis.configuration_values - self.assertIn("n_iterations", cfg.keys()) + with self.assertRaises(AttributeError): + analysis.furry_purry_cat diff --git a/tests/test_runner.py b/tests/test_runner.py index 5531ef068..d509b12f0 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -3,6 +3,10 @@ from .base import BaseTestCase +def mock_app(analysis): + pass + + class RunnerTestCase(BaseTestCase): def test_instantiate_runner(self): """ Ensures that runner whose twine requires configuration can be instantiated @@ -27,10 +31,7 @@ def test_run_with_configuration_passes(self): configuration_values="{}", ) - def fcn(analysis): - pass - - runner.run(fcn) + runner.run(mock_app) def test_instantiation_without_configuration_fails(self): """ Ensures that runner can be instantiated with a string that points to a path @@ -72,11 +73,8 @@ def test_run_output_values_validation(self): ) # Test for failure with an incorrect output - def fcn(analysis): - pass - with self.assertRaises(twined.exceptions.InvalidValuesContents) as error: - runner.run(fcn).finalise() + runner.run(mock_app).finalise() self.assertIn("'n_iterations' is a required property", error.exception.args[0]) @@ -112,13 +110,32 @@ def test_exception_raised_when_strand_data_missing(self): configuration_values={"n_iterations": 5}, ) - def fcn(analysis): - pass - with self.assertRaises(twined.exceptions.TwineValueException) as error: - runner.run(fcn) + runner.run(mock_app) self.assertIn( "The 'input_values' strand is defined in the twine, but no data is provided in sources", error.exception.args[0], ) + + def test_output_manifest_is_not_none(self): + """ Ensure the output manifest of an analysis is not None if an output manifest is defined in the Twine. """ + runner = Runner( + twine=""" + { + "output_manifest": [ + { + "key": "open_foam_result", + "purpose": "A dataset containing solution fields of an openfoam case." + }, + { + "key": "airfoil_cp_values", + "purpose": "A file containing cp values" + } + ] + } + """ + ) + + analysis = runner.run(mock_app) + self.assertIsNotNone(analysis.output_manifest)