From fffece4a6fb265faabd98a8f187307cdc5440ece Mon Sep 17 00:00:00 2001 From: Henry Wright <84939917+HGWright@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:58:59 +0000 Subject: [PATCH] Accepting Ruff, MyPy, NumPydoc pre-commit checks (#288) * add checks and exceptions * remove .flake8 file * add citations to ignored checks * review comments and rerun repo review * add extra links --- .flake8 | 28 --- .github/ISSUE_TEMPLATE/bug-report.md | 8 +- .github/workflows/ci-wheels.yml | 2 +- .github/workflows/stale.yml | 8 +- .gitignore | 2 +- .pre-commit-config.yaml | 72 ++++++-- LICENSE | 3 +- README.md | 1 - pyproject.toml | 203 ++++++++++++++++++++- requirements/pypi-core.txt | 2 +- setup.py | 2 +- src/stratify/__init__.py | 2 +- src/stratify/_bounded_vinterp.py | 11 +- src/stratify/tests/performance.py | 5 +- src/stratify/tests/test_bounded_vinterp.py | 4 +- src/stratify/tests/test_vinterp.py | 8 +- 16 files changed, 276 insertions(+), 85 deletions(-) delete mode 100644 .flake8 diff --git a/.flake8 b/.flake8 deleted file mode 100644 index dfb1021..0000000 --- a/.flake8 +++ /dev/null @@ -1,28 +0,0 @@ -[flake8] - -# References: -# https://flake8.readthedocs.io/en/latest/user/configuration.html -# https://flake8.readthedocs.io/en/latest/user/error-codes.html -# https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes - -select = C,D,E,F,W,B,B950 -ignore = - # E203: whitespace before ':' - E203, - # E226: missing whitespace around arithmetic operator - E226, - # E231: missing whitespace after ',', ';', or ':' - E231, - # E402: module level imports on one line - E402, - # E501: line too long - E501, - # E731: do not assign a lambda expression, use a def - E731, - # W503: line break before binary operator - W503, - # W504: line break after binary operator - W504 -exclude = - .eggs - build diff --git a/.github/ISSUE_TEMPLATE/bug-report.md b/.github/ISSUE_TEMPLATE/bug-report.md index c7031f4..2eec255 100644 --- a/.github/ISSUE_TEMPLATE/bug-report.md +++ b/.github/ISSUE_TEMPLATE/bug-report.md @@ -13,9 +13,9 @@ assignees: '' ## How To Reproduce Steps to reproduce the behaviour: -1. -2. -3. +1. +2. +3. ## Expected behaviour @@ -23,7 +23,7 @@ Steps to reproduce the behaviour: ## Screenshots -## Environment +## Environment - OS & Version: [e.g., Ubuntu 20.04 LTS] - Stratify Version diff --git a/.github/workflows/ci-wheels.yml b/.github/workflows/ci-wheels.yml index f0cdd69..6d93f50 100644 --- a/.github/workflows/ci-wheels.yml +++ b/.github/workflows/ci-wheels.yml @@ -76,7 +76,7 @@ jobs: - name: "Building sdist" shell: bash run: | - pipx run build --sdist + pipx run build --sdist - uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index 8f50cf8..f00ddfe 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -38,7 +38,7 @@ jobs: Otherwise this issue will be automatically closed in ${{ env.DAYS_BEFORE_CLOSE }} days time. # Comment on the staled prs. - stale-pr-message: | + stale-pr-message: | In order to maintain a backlog of relevant PRs, we automatically label them as stale after ${{ env.DAYS_BEFORE_STALE }} days of inactivity. If this PR is still important to you, then please comment on this PR and the stale label will be removed. @@ -48,7 +48,7 @@ jobs: # Comment on the staled issues while closed. close-issue-message: | This stale issue has been automatically closed due to a lack of community activity. - + If you still care about this issue, then please either: * Re-open this issue, if you have sufficient permissions, or * Add a comment pinging `@SciTools/stratify-maintainers` who will re-open on your behalf. @@ -56,7 +56,7 @@ jobs: # Comment on the staled prs while closed. close-pr-message: | This stale PR has been automatically closed due to a lack of community activity. - + If you still care about this PR, then please either: * Re-open this PR, if you have sufficient permissions, or * Add a comment pinging `@SciTools/stratify-maintainers` who will re-open on your behalf. @@ -85,4 +85,4 @@ jobs: ascending: true # Exempt all issues/prs with milestones from stale. - exempt-all-milestones: true \ No newline at end of file + exempt-all-milestones: true diff --git a/.gitignore b/.gitignore index b44dff9..e27ee1f 100644 --- a/.gitignore +++ b/.gitignore @@ -133,4 +133,4 @@ dmypy.json .pyre/ # PyCharm -.idea \ No newline at end of file +.idea diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 620072d..b3d6652 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,8 +1,11 @@ # See https://pre-commit.com for more information # See https://pre-commit.com/hooks.html for more hooks +# See https://pre-commit.ci/#configuration +# See https://github.com/scientific-python/cookie#sp-repo-review ci: - autoupdate_commit_msg: "chore: update pre-commit hooks" + autofix_prs: false + autoupdate_commit_msg: "chore: update pre-commit hooks" repos: - repo: https://github.com/pre-commit/pre-commit-hooks @@ -18,27 +21,43 @@ repos: - id: check-merge-conflict # Check for debugger imports and py37+ `breakpoint()` calls in Python source. - id: debug-statements + # Check TOML file syntax. + - id: check-toml + # Check YAML file syntax. + - id: check-yaml + # Makes sure files end in a newline and only a newline. + # Duplicates Ruff W292 but also works on non-Python files. + - id: end-of-file-fixer + # Replaces or checks mixed line ending. + - id: mixed-line-ending # Don't commit to master branch. - id: no-commit-to-branch -- repo: https://github.com/psf/black-pre-commit-mirror - rev: '24.10.0' + # Trims trailing whitespace. + # Duplicates Ruff W291 but also works on non-Python files. + - id: trailing-whitespace + + +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: "v0.7.3" hooks: - - id: black + - id: ruff + types: [file, python] + args: [--fix, --show-fixes] + - id: ruff-format types: [file, python] - args: [--config=./pyproject.toml, .] -- repo: https://github.com/PyCQA/flake8 - rev: '7.1.1' + +- repo: https://github.com/codespell-project/codespell + rev: "v2.3.0" hooks: - # Run flake8. - - id: flake8 - args: [--config=./.flake8] + - id: codespell + types_or: [asciidoc, python, markdown, rst] + additional_dependencies: [tomli] + + + - repo: https://github.com/pycqa/isort rev: '5.13.2' hooks: - - id: isort - name: isort (python) - types: [file, python] - args: [--filter-files] - id: isort name: isort (cython) types: [file, cython] @@ -49,3 +68,28 @@ repos: hooks: - id: sort-all types: [file, python] + +- repo: https://github.com/pre-commit/mirrors-mypy + rev: 'v1.13.0' + hooks: + - id: mypy + exclude: 'noxfile\.py|docs/conf\.py' + +- repo: https://github.com/abravalheri/validate-pyproject + # More exhaustive than Ruff RUF200. + rev: "v0.23" + hooks: + - id: validate-pyproject + +- repo: https://github.com/scientific-python/cookie + rev: 2024.08.19 + hooks: + - id: sp-repo-review + additional_dependencies: ["repo-review[cli]"] + args: ["--show=errskip"] + +- repo: https://github.com/numpy/numpydoc + rev: v1.8.0 + hooks: + - id: numpydoc-validation + types: [file, python] diff --git a/LICENSE b/LICENSE index 9481f6f..a78b1ef 100644 --- a/LICENSE +++ b/LICENSE @@ -1,5 +1,5 @@ BSD 3-Clause License -Copyright (c) 2015 - 2017, Met Office. +Copyright (c) 2015 - 2024, Met Office. All rights reserved. Redistribution and use in source and binary forms, with or without @@ -26,4 +26,3 @@ SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - diff --git a/README.md b/README.md index a01ace3..b63bad3 100644 --- a/README.md +++ b/README.md @@ -33,4 +33,3 @@ pip install stratify ## License Stratify is licenced under a [BSD 3-Clause License](LICENSE). - diff --git a/pyproject.toml b/pyproject.toml index 841a3aa..ff25ca2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -68,16 +68,6 @@ where = ["src"] write_to = "src/stratify/_version.py" local_scheme = "dirty-tag" -[tool.black] -target-version = ["py310", "py311", "py312"] -line-length = 88 -include = '\.pyi?$' - -[tool.isort] -profile = "black" -skip_gitignore = "True" -verbose = "True" - [tool.check-manifest] ignore = [ "src/stratify/_conservative.c", @@ -88,3 +78,196 @@ ignore = [ [tool.pytest.ini_options] addopts = "-ra -v --doctest-modules" testpaths = ["src/stratify"] + +[tool.mypy] +disable_error_code = ["call-arg", "no-untyped-def", "no-untyped-call", "attr-defined", "misc", "index", "var-annotated", "assignment"] +enable_error_code = ["ignore-without-code", "truthy-bool", "redundant-expr"] +warn_unreachable = true +strict = true + +[tool.numpydoc_validation] +checks = [ + "all", # Enable all numpydoc validation rules, apart from the following: + + # -> Docstring text (summary) should start in the line immediately + # after the opening quotes (not in the same line, or leaving a + # blank line in between) + "GL01", # Permit summary line on same line as docstring opening quotes. + + # -> Closing quotes should be placed in the line after the last text + # in the docstring (do not close the quotes in the same line as + # the text, or leave a blank line between the last text and the + # quotes) + "GL02", # Permit a blank line before docstring closing quotes. + + # -> Double line break found; please use only one blank line to + # separate sections or paragraphs, and do not leave blank lines + # at the end of docstrings + "GL03", # Ignoring. + + "GL08", # The object does not have a docstring + + "PR01", # Parameters {missing_params} not documented + "PR02", # Unknown parameters {unknown_params} + "PR10", # Parameter "{param_name}" requires a space before the colon separating the parameter name and type + + "RT04", # Return value description should start with a capital letter + + "SS06", # Summary should fit in a single line + + + # -> See Also section not found + "SA01", # Not all docstrings require a "See Also" section. + + # -> No extended summary found + "ES01", # Not all docstrings require an "Extended Summary" section. + + # -> No examples section found + "EX01", # Not all docstrings require an "Examples" section. + + # -> No Yields section found + "YD01", # Not all docstrings require a "Yields" section. + + # Record temporarily ignored checks below; will be reviewed at a later date: + +] +exclude = [ + '\.__eq__$', + '\.__ne__$', + '\.__repr__$', +] + +[tool.ruff] +line-length = 88 + +[tool.ruff.format] +preview = false + +[tool.ruff.lint] +ignore = [ + + # flake8-annotations (ANN) + # https://docs.astral.sh/ruff/rules/#flake8-annotations-ann + "ANN001", # Missing type annotation for function argument {name} + "ANN002", # Missing type annotation for *{name} + "ANN003", # Missing type annotation for **{name} + "ANN201", # Missing return type annotation for public function {name} + "ANN202", # Missing return type annotation for private function {name} + "ANN204", # Missing return type annotation for special method {name} + + "ARG002", # Unused method argument: {name} + "ARG003", # Unused class method argument: {name} + + # flake8-bugbear (B) + # https://docs.astral.sh/ruff/rules/#flake8-bugbear-b + "B028", # No explicit stacklevel keyword argument found + + # flake8-comprehensions (C4) + # https://docs.astral.sh/ruff/rules/#flake8-comprehensions-c4 + "C405", # Unnecessary {obj_type} literal (rewrite as a set literal) + "C419", # Unnecessary list comprehension + + # flake8-commas (COM) + # https://docs.astral.sh/ruff/rules/#flake8-commas-com + "COM812", # Trailing comma missing. + "COM819", # Trailing comma prohibited. + + # pydocstyle (D) + # https://docs.astral.sh/ruff/rules/#pydocstyle-d + "D100", # Missing docstring in public module + "D101", # Missing docstring in public class + "D102", # Missing docstring in public method + "D103", # Missing docstring in public function + "D104", # Missing docstring in public package + "D106", # Missing docstring in public nested class + "D205", # 1 blank line required between summary line and description + + # https://docs.astral.sh/ruff/rules/#eradicate-era + "ERA001", # Found commented-out code + + # flake8-boolean-trap (FBT) + # https://docs.astral.sh/ruff/rules/#flake8-boolean-trap-fbt + "FBT002", # Boolean default positional argument in function definition + + # flake8-implicit-str-concat (ISC) + # https://docs.astral.sh/ruff/rules/single-line-implicit-string-concatenation/ + # NOTE: This rule may cause conflicts when used with "ruff format". + "ISC001", # Implicitly concatenate string literals on one line. + + # pep8-naming (N) + # https://docs.astral.sh/ruff/rules/#pep8-naming-n + "N801", # Class name {name} should use CapWords convention + + # Refactor (R) + # https://docs.astral.sh/ruff/rules/#refactor-r + "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable + + # flake8-pytest-style (PT) + # https://docs.astral.sh/ruff/rules/#flake8-pytest-style-pt + "PT009", # Use a regular assert instead of unittest-style {assertion} + "PT027", # Use pytest.raises instead of unittest-style {assertion} + + # flake8-return (RET) + # https://docs.astral.sh/ruff/rules/#flake8-return-ret + "RET504", # Unnecessary assignment to {name} before return statement + + # Ruff-specific rules (RUF) + # https://docs.astral.sh/ruff/rules/#ruff-specific-rules-ruf + "RUF005", # Consider {expression} instead of concatenation + "RUF012", # Mutable class attributes should be annotated with typing.ClassVar + + # flake8-self (SLF) + # https://docs.astral.sh/ruff/rules/#flake8-self-slf + "SLF001", # Private member accessed: {access} + + # flake8-print (T20) + # https://docs.astral.sh/ruff/rules/#flake8-print-t20 + "T201", # print found + + ] + preview = false + select = [ + "ALL", + # list specific rules to include that is skipped using numpy convention. + "D212", # Multi-line docstring summary should start at the first line + ] + +[tool.ruff.lint.isort] +force-sort-within-sections = true +# Change to match specific package name: +known-first-party = ["iris"] + +[tool.ruff.lint.per-file-ignores] +# All test scripts + +# Change to match specific package path: +"lib/iris/tests/*.py" = [ + # https://docs.astral.sh/ruff/rules/undocumented-public-module/ + "D100", # Missing docstring in public module + "D205", # 1 blank line required between summary line and description + "D401", # 1 First line of docstring should be in imperative mood +] + +[tool.ruff.lint.pydocstyle] +convention = "numpy" + +[tool.codespell] +ignore-words-list = "ND,Nd" + +[tool.repo-review] +# These are a list of the currently failing tests to be fixed later: +ignore = [ + "PY004", # Has docs folder + "PP302", # Sets a minimum pytest to at least 6 + "PP304", # Sets the log level in pytest + "PP305", # Specifies xfail_strict + "PP306", # Specifies strict config + "PP307", # Specifies strict markers + "PP309", # Filter warnings specified + "PY007", # Supports an easy task runner (nox or tox) + "PP003", # Does not list wheel as a build-dep + "PC111", # Uses blacken-docs + "PC170", # Uses PyGrep hooks (only needed if rST present) + "PC180", # Uses a markdown formatter + "RTD100" # Uses ReadTheDocs (pyproject config) +] diff --git a/requirements/pypi-core.txt b/requirements/pypi-core.txt index d7724f1..0cadfc3 100644 --- a/requirements/pypi-core.txt +++ b/requirements/pypi-core.txt @@ -1,3 +1,3 @@ Cython dask[array] -numpy==1.26.4 \ No newline at end of file +numpy==1.26.4 diff --git a/setup.py b/setup.py index bc27702..964cba9 100644 --- a/setup.py +++ b/setup.py @@ -1,7 +1,7 @@ import os +from pathlib import Path import sys import warnings -from pathlib import Path # safe to import numpy here thanks to pep518 import numpy as np diff --git a/src/stratify/__init__.py b/src/stratify/__init__.py index 8427632..8e8d518 100644 --- a/src/stratify/__init__.py +++ b/src/stratify/__init__.py @@ -1,3 +1,3 @@ from ._bounded_vinterp import interpolate_conservative # noqa: F401 from ._version import version as __version__ # noqa: F401 -from ._vinterp import * # noqa: F401, F403 +from ._vinterp import * # noqa: F403 diff --git a/src/stratify/_bounded_vinterp.py b/src/stratify/_bounded_vinterp.py index d19c85e..465a96d 100644 --- a/src/stratify/_bounded_vinterp.py +++ b/src/stratify/_bounded_vinterp.py @@ -4,8 +4,7 @@ def interpolate_conservative(z_target, z_src, fz_src, axis=-1): - """ - 1d conservative interpolation across multiple dimensions. + """1d conservative interpolation across multiple dimensions. This function provides the ability to perform 1d interpolation on datasets with more than one dimension. For instance, this function can be used to @@ -106,9 +105,9 @@ def interpolate_conservative(z_target, z_src, fz_src, axis=-1): fz_src_reshaped = np.transpose(fz_src, data_transpose) fz_src_orig = list(fz_src_reshaped.shape) shape = ( - int(np.product(fz_src_reshaped.shape[: len(bdims)])), + int(np.prod(fz_src_reshaped.shape[: len(bdims)])), fz_src_reshaped.shape[len(bdims)], - int(np.product(fz_src_reshaped.shape[len(bdims) + 1 :])), + int(np.prod(fz_src_reshaped.shape[len(bdims) + 1 :])), ) fz_src_reshaped = fz_src_reshaped.reshape(shape) @@ -118,11 +117,11 @@ def interpolate_conservative(z_target, z_src, fz_src, axis=-1): z_src_reshaped = np.transpose(z_src, [axis_relative] + vdims) z_target_reshaped = np.transpose(z_target, [axis_relative] + vdims) - shape = int(np.product(z_src_reshaped.shape[1:-1])) + shape = int(np.prod(z_src_reshaped.shape[1:-1])) z_src_reshaped = z_src_reshaped.reshape( [z_src_reshaped.shape[0], shape, z_src_reshaped.shape[-1]] ) - shape = int(np.product(z_target_reshaped.shape[1:-1])) + shape = int(np.prod(z_target_reshaped.shape[1:-1])) z_target_reshaped = z_target_reshaped.reshape( [z_target_reshaped.shape[0], shape, z_target_reshaped.shape[-1]] ) diff --git a/src/stratify/tests/performance.py b/src/stratify/tests/performance.py index e91e190..88e6616 100644 --- a/src/stratify/tests/performance.py +++ b/src/stratify/tests/performance.py @@ -1,7 +1,4 @@ -""" -Functions that may be used to measure performance of a component. - -""" +"""Functions that may be used to measure performance of a component.""" import dask.array as da import numpy as np diff --git a/src/stratify/tests/test_bounded_vinterp.py b/src/stratify/tests/test_bounded_vinterp.py index 3754949..06ba948 100644 --- a/src/stratify/tests/test_bounded_vinterp.py +++ b/src/stratify/tests/test_bounded_vinterp.py @@ -54,7 +54,7 @@ def test_no_broadcasting(self): res = bounded_vinterp.interpolate_conservative( target_bounds, self.bounds, data, axis=0 ) - target_data = np.ones((3)) * 2 + target_data = np.ones(3) * 2 assert_array_equal(res, target_data) def test_source_with_nans(self): @@ -67,7 +67,7 @@ def test_source_with_nans(self): res = bounded_vinterp.interpolate_conservative( target_bounds, self.bounds, data, axis=0 ) - target_data = np.ones((12)) * 0.5 + target_data = np.ones(12) * 0.5 target_data[:2] = np.nan target_data[-4:] = np.nan assert_array_equal(res, target_data) diff --git a/src/stratify/tests/test_vinterp.py b/src/stratify/tests/test_vinterp.py index 36b7932..002bbc4 100644 --- a/src/stratify/tests/test_vinterp.py +++ b/src/stratify/tests/test_vinterp.py @@ -410,7 +410,7 @@ def test_axis_out_of_bounds_z_src_absolute(self): def test_axis_greater_than_z_src_ndim(self): # Ensure that axis is not unnecessarily constrained to the dimensions # of z_src. - data = np.empty((4)) + data = np.empty(4) zdata = np.empty((3, 5, 4)) axis = 2 result = vinterp._Interpolation(data.copy(), data, zdata, axis=axis) @@ -480,8 +480,7 @@ def test_target_z_3d_on_axis_m1(self): assert_array_equal(result, f_source) def test_target_z_2d_over_3d_on_axis_1(self): - """ - Test the case where z_target(2, 4) and z_src(3, 4) are 2d, but the + """Test the case where z_target(2, 4) and z_src(3, 4) are 2d, but the source data fz_src(3, 3, 4) is 3d. z_target and z_src cover the last 2 dimensions of fz_src. The axis of interpolation is axis=1 wrt fz_src. @@ -506,8 +505,7 @@ def test_target_z_2d_over_3d_on_axis_1(self): assert_array_equal(result, expected) def test_target_z_2d_over_3d_on_axis_m1(self): - """ - Test the case where z_target(3, 3) and z_src(3, 4) are 2d, but the + """Test the case where z_target(3, 3) and z_src(3, 4) are 2d, but the source data fz_src(3, 3, 4) is 3d. z_target and z_src cover the last 2 dimensions of fz_src. The axis of interpolation is the default last dimension, axis=-1, wrt fx_src.