From 4f0c26d1b4a01ab4ac122f0bf616e17786ceeade Mon Sep 17 00:00:00 2001 From: cortadocodes Date: Thu, 9 Dec 2021 15:45:27 +0000 Subject: [PATCH 1/2] STY: Add and apply pep8-naming flake8 extension to pre-commit --- .pre-commit-config.yaml | 4 ++++ tests/test_manifest_strands.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 65a2d61..409119d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,6 +30,10 @@ repos: hooks: - id: flake8 language_version: python3 + additional_dependencies: + - 'pep8-naming' + args: + - --ignore-names=setUp,tearDown,setUpClass,tearDownClass,asyncSetUp,asyncTearDown,setUpTestData,failureException,longMessage,maxDiff,startTestRun,stopTestRun - repo: https://github.com/thclark/pre-commit-sphinx rev: 0.0.1 diff --git a/tests/test_manifest_strands.py b/tests/test_manifest_strands.py index c42a291..a879e15 100644 --- a/tests/test_manifest_strands.py +++ b/tests/test_manifest_strands.py @@ -478,7 +478,7 @@ def patch_if_url_is_schema_url(instance, url): def test_validate_input_manifest_with_required_tags_in_several_datasets(self): """Test that required tags from the file tags template are validated separately and correctly for each dataset.""" - TWINE_WITH_INPUT_MANIFEST_WITH_REQUIRED_TAGS_FOR_MULTIPLE_DATASETS = """ + twine_with_input_manifest_with_required_tags_for_multiple_datasets = """ { "input_manifest": { "datasets": [ @@ -566,7 +566,7 @@ def test_validate_input_manifest_with_required_tags_in_several_datasets(self): } """ - twine = Twine(source=TWINE_WITH_INPUT_MANIFEST_WITH_REQUIRED_TAGS_FOR_MULTIPLE_DATASETS) + twine = Twine(source=twine_with_input_manifest_with_required_tags_for_multiple_datasets) twine.validate_input_manifest(source=input_manifest) def test_error_raised_if_multiple_datasets_have_same_name(self): From a9db7abe165ed978e8282115c3fe6fdc77157074 Mon Sep 17 00:00:00 2001 From: cortadocodes Date: Thu, 9 Dec 2021 15:53:51 +0000 Subject: [PATCH 2/2] OPS: Add and apply docstring linting --- .pre-commit-config.yaml | 5 ++++ setup.cfg | 3 +++ setup.py | 2 +- tests/base.py | 4 +++ tests/test_children.py | 2 +- tests/test_credentials.py | 3 ++- twined/twine.py | 53 ++++++++++++++------------------------- twined/utils/encoders.py | 6 ++++- twined/utils/load_json.py | 2 +- 9 files changed, 41 insertions(+), 39 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 409119d..f56d8df 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,6 +35,11 @@ repos: args: - --ignore-names=setUp,tearDown,setUpClass,tearDownClass,asyncSetUp,asyncTearDown,setUpTestData,failureException,longMessage,maxDiff,startTestRun,stopTestRun + - repo: https://github.com/pycqa/pydocstyle + rev: 6.1.1 + hooks: + - id: pydocstyle + - repo: https://github.com/thclark/pre-commit-sphinx rev: 0.0.1 hooks: diff --git a/setup.cfg b/setup.cfg index 1dc4ea1..c32c7d6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -22,3 +22,6 @@ multi_line_output=3 include_trailing_comma=True force_grid_wrap=0 combine_as_imports=True + +[pydocstyle] +ignore = D100, D101, D104, D105, D107, D203, D205, D213, D301, D400, D415 diff --git a/setup.py b/setup.py index 5fa88e3..145f62e 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ setup( name="twined", - version="0.1.0", + version="0.1.1", py_modules=[], install_requires=["jsonschema ~= 3.2.0", "python-dotenv"], url="https://www.github.com/octue/twined", diff --git a/tests/base.py b/tests/base.py index 2df2e99..48a229f 100644 --- a/tests/base.py +++ b/tests/base.py @@ -56,6 +56,10 @@ class BaseTestCase(unittest.TestCase): """ def setUp(self): + """Add the tests data directory to the test class as an attribute. + + :return None: + """ self.path = os.path.join(os.path.dirname(__file__), "data") super().setUp() diff --git a/tests/test_children.py b/tests/test_children.py index f2b5968..5f47c2d 100644 --- a/tests/test_children.py +++ b/tests/test_children.py @@ -82,7 +82,7 @@ def test_extra_children(self): Twine().validate_children(source=self.VALID_CHILD_VALUE) def test_backend_cannot_be_empty(self): - """ Test that the backend field of a child cannot be empty. """ + """Test that the backend field of a child cannot be empty.""" single_child_missing_backend = """[{"key": "gis", "id": "some-id", "backend": {}}]""" with self.assertRaises(exceptions.InvalidValuesContents): diff --git a/tests/test_credentials.py b/tests/test_credentials.py index 34253d7..e3f27ad 100644 --- a/tests/test_credentials.py +++ b/tests/test_credentials.py @@ -63,6 +63,7 @@ def test_fails_on_dict(self): Twine(source=invalid_credentials_dict_not_array_twine) def test_fails_on_name_whitespace(self): + """Test that a credential with spaces in its name causes an error to be raised when validated.""" invalid_credentials_space_in_name_twine = """ { "credentials": [ @@ -111,7 +112,7 @@ def test_missing_credentials(self): twine.validate_credentials() def test_credentials(self): - """ Test that the environment will override a default value for a credential.""" + """Test that the environment will override a default value for a credential.""" twine = Twine(source=self.VALID_CREDENTIALS_TWINE) with mock.patch.dict( os.environ, diff --git a/twined/twine.py b/twined/twine.py index 27d53a5..34e497d 100644 --- a/twined/twine.py +++ b/twined/twine.py @@ -40,19 +40,16 @@ class Twine: Note: Instantiating the twine does not validate that any inputs to an application are correct - it merely checks that the twine itself is correct. - """ def __init__(self, **kwargs): - """Constructor for the twine class""" for name, strand in self._load_twine(**kwargs).items(): setattr(self, name, strand) self._available_strands = tuple(trim_suffix(name, "_schema") for name in vars(self)) def _load_twine(self, source=None): - """Load twine from a *.json filename, file-like or a json string and validates twine contents""" - + """Load twine from a *.json filename, file-like or a json string and validates twine contents.""" if source is None: # If loading an unspecified twine, return an empty one rather than raising error (like in _load_data()) raw = {} @@ -65,8 +62,7 @@ def _load_twine(self, source=None): return raw def _load_json(self, kind, source, **kwargs): - """Loads data from either a *.json file, an open file pointer or a json string. Directly returns any other data""" - + """Load data from either a *.json file, an open file pointer or a json string. Directly returns any other data.""" if source is None: raise exceptions.invalid_json_map[kind](f"Cannot load {kind} - no data source specified.") @@ -124,7 +120,7 @@ def _get_schema(self, strand): return jsonlib.loads(pkg_resources.resource_string("twined", schema_path)) def _validate_against_schema(self, strand, data): - """Validates data against a schema, raises exceptions of type InvalidJson if not compliant. + """Validate data against a schema, raises exceptions of type InvalidJson if not compliant. Can be used to validate: - the twine file contents itself against the present version twine spec @@ -145,7 +141,7 @@ def _validate_against_schema(self, strand, data): raise exceptions.invalid_contents_map[strand](str(e)) def _validate_twine_version(self, twine_file_twined_version): - """Validates that the installed version is consistent with an optional version specification in the twine file""" + """Validate that the installed version is consistent with an optional version specification in the twine file.""" installed_twined_version = pkg_resources.get_distribution("twined").version logger.debug( "Twine versions... %s installed, %s specified in twine", installed_twined_version, twine_file_twined_version @@ -156,7 +152,7 @@ def _validate_twine_version(self, twine_file_twined_version): ) def _validate_values(self, kind, source, cls=None, **kwargs): - """Common values validator method""" + """Validate values against the twine schema.""" data = self._load_json(kind, source, **kwargs) self._validate_against_schema(kind, data) if cls: @@ -164,7 +160,7 @@ def _validate_values(self, kind, source, cls=None, **kwargs): return data def _validate_manifest(self, kind, source, cls=None, **kwargs): - """Common manifest validator method""" + """Validate manifest against the twine schema.""" data = self._load_json(kind, source, **kwargs) # TODO elegant way of cleaning up this nasty serialisation hack to manage conversion of outbound manifests to primitive @@ -224,7 +220,7 @@ def available_strands(self): return self._available_strands def validate_children(self, source, **kwargs): - """Validates that the children values, passed as either a file or a json string, are correct""" + """Validate that the children values, passed as either a file or a json string, are correct.""" # TODO cache this loaded data keyed on a hashed version of kwargs children = self._load_json("children", source, **kwargs) self._validate_against_schema("children", children) @@ -302,15 +298,15 @@ def validate_credentials(self, *args, dotenv_path=None, **kwargs): return self.credentials def validate_configuration_values(self, source, **kwargs): - """Validates that the configuration values, passed as either a file or a json string, are correct""" + """Validate that the configuration values, passed as either a file or a json string, are correct.""" return self._validate_values("configuration_values", source, **kwargs) def validate_input_values(self, source, **kwargs): - """Validates that the input values, passed as either a file or a json string, are correct""" + """Validate that the input values, passed as either a file or a json string, are correct.""" return self._validate_values("input_values", source, **kwargs) def validate_output_values(self, source, **kwargs): - """Validates that the output values, passed as either a file or a json string, are correct""" + """Validate that the output values, passed as either a file or a json string, are correct.""" return self._validate_values("output_values", source, **kwargs) def validate_monitor_message(self, source, **kwargs): @@ -318,15 +314,15 @@ def validate_monitor_message(self, source, **kwargs): return self._validate_values(kind="monitor_message", source=source, **kwargs) def validate_configuration_manifest(self, source, **kwargs): - """Validates the input manifest, passed as either a file or a json string""" + """Validate the input manifest, passed as either a file or a json string.""" return self._validate_manifest("configuration_manifest", source, **kwargs) def validate_input_manifest(self, source, **kwargs): - """Validates the input manifest, passed as either a file or a json string""" + """Validate the input manifest, passed as either a file or a json string.""" return self._validate_manifest("input_manifest", source, **kwargs) def validate_output_manifest(self, source, **kwargs): - """Validates the output manifest, passed as either a file or a json string""" + """Validate the output manifest, passed as either a file or a json string.""" return self._validate_manifest("output_manifest", source, **kwargs) @staticmethod @@ -350,22 +346,11 @@ def validate(self, allow_missing=False, allow_extra=False, cls=None, **kwargs): ) ``` - :parameter allow_missing: If strand is present in the twine, but the source is equal to None, allow validation - to continue. - :type allow_missing: bool - - :parameter allow_extra: If strand is present in the sources, but not in the twine, allow validation to continue - (only strands in the twine will be validated and converted, others will be returned as-is) - :type allow_extra: bool - - :parameter cls: optional dict of classes keyed on strand name (alternatively, one single class which will be - applied to strands) which will be instantiated with the validated source data. - :type cls: dict or any - - :return: dict of validated and initialised sources - :rtype: dict + :param bool allow_missing: If strand is present in the twine, but the source is equal to None, allow validation to continue. + :param bool allow_extra: If strand is present in the sources, but not in the twine, allow validation to continue (only strands in the twine will be validated and converted, others will be returned as-is) + :param any cls: optional dict of classes keyed on strand name (alternatively, one single class which will be applied to strands) which will be instantiated with the validated source data. + :return dict: dict of validated and initialised sources """ - # pop any strand name:data pairs out of kwargs and into their own dict source_kwargs = tuple(name for name in kwargs.keys() if name in ALL_STRANDS) sources = dict((name, kwargs.pop(name)) for name in source_kwargs) @@ -401,11 +386,11 @@ def validate(self, allow_missing=False, allow_extra=False, cls=None, **kwargs): return sources def validate_strand(self, name, source, **kwargs): - """Validates a single strand by name""" + """Validate a single strand by name.""" return self.validate({name: source}, **kwargs)[name] def prepare(self, *args, cls=None, **kwargs): - """Prepares instance for strand data using a class map""" + """Prepare instance for strand data using a class map.""" prepared = {} for arg in args: if arg not in ALL_STRANDS: diff --git a/twined/utils/encoders.py b/twined/utils/encoders.py index e9749dc..2ec6825 100644 --- a/twined/utils/encoders.py +++ b/twined/utils/encoders.py @@ -19,10 +19,14 @@ class TwinedEncoder(json.JSONEncoder): some_json = {"a": np.array([0, 1])} json.dumps(some_json, cls=TwinedEncoder) ``` - """ def default(self, obj): + """Convert the given object to python primitives. + + :param any obj: + :return any: + """ if _numpy_spec is not None: import numpy diff --git a/twined/utils/load_json.py b/twined/utils/load_json.py index 1290c98..264708a 100644 --- a/twined/utils/load_json.py +++ b/twined/utils/load_json.py @@ -12,7 +12,7 @@ def load_json(source, *args, **kwargs): - """Loads json, automatically detecting whether the input is a valid filename, a string containing json data, + """Load JSON, automatically detecting whether the input is a valid filename, a string containing json data, or a python dict already (in which case the result is returned directly). That makes this function suitable for use in a pipeline where it's not clear whether data has been loaded yet, or