Skip to content

Commit

Permalink
MRG: Merge pull request #93 from octue/devops/add-docstring-linting
Browse files Browse the repository at this point in the history
Add docstring linting and PEP8 naming pre-commit checks
  • Loading branch information
cortadocodes authored Dec 9, 2021
2 parents 0917bc1 + a9db7ab commit ae7cef4
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 41 deletions.
9 changes: 9 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion tests/test_children.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
3 changes: 2 additions & 1 deletion tests/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions tests/test_manifest_strands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -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):
Expand Down
53 changes: 19 additions & 34 deletions twined/twine.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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.")

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion twined/utils/encoders.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion twined/utils/load_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ae7cef4

Please sign in to comment.