MRG: Merge pull request #93 from octue/devops/add-docstring-linting

Add docstring linting and PEP8 naming pre-commit checks

.pre-commit-config.yaml

@@ -30,6 +30,15 @@ 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/pycqa/pydocstyle
+    rev: 6.1.1
+    hooks:
+    - id: pydocstyle
 
   - repo: https://github.com/thclark/pre-commit-sphinx
     rev: 0.0.1

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

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",

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()
 

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):

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,

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):

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 Invalid<strand>Json if not compliant.
+        """Validate data against a schema, raises exceptions of type Invalid<strand>Json 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,15 +152,15 @@ 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:
             return cls(**data)
         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,31 +298,31 @@ 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):
         """Validate monitor message against the monitor message schema strand."""
         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:

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
 

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