Add pre-commit to CI and fix all typing and linting (#306)

* Add pre-commit * Add relint file * Auto-linting * Auto-linting . files * Reactivating black, starting flake8 * Fixing codespell * Black reformatting * Fix flake8 issues * Incremental commit on mypy * Incremental commit on mypy * Incremental commit in mypy * Incremental commit on mypy * Incremental commit on mypy * Finish mypy, add isort and pyupgrade * Rerun all * black and isort are fighting! * Make isort profile for compatibility with black * Import annotations in setup * Fix typing import * Modify definition of NDArrayf for backward compatibility * Make typing backward compatible for Python 3.8 and before * Change typing with Python version * Remove type subscription before Python 3.9 * Fix choice order function and fitting tests * Fix test errors due to linting * Final linting * Fix isinstance syntax for python 3.8
GlacioHack · Sep 20, 2022 · 69a1044 · 69a1044
1 parent 60fed0f
commit 69a1044
Show file tree

Hide file tree

Showing 76 changed files with 4,394 additions and 3,016 deletions.
diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml
@@ -0,0 +1,15 @@
+name: Linting and formatting (pre-commit)
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-python@v2
+    - uses: pre-commit/action@v2.0.0
diff --git a/.github/workflows/python-publish.yml b/.github/workflows/python-publish.yml
@@ -10,9 +10,9 @@ on:
   workflow_dispatch:
      inputs:
         reason:
-          description: 'Reason for manual trigger'     
+          description: 'Reason for manual trigger'
           required: true
-          default: 'testing' 
+          default: 'testing'
 
 jobs:
   deploy:

diff --git a/.gitignore b/.gitignore
@@ -148,4 +148,4 @@ examples/data/*.csv
 docs/source/basic_examples/
 docs/source/advanced_examples/
 docs/source/gen_modules/
-examples/basic/temp.tif
+examples/basic/temp.tif
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,113 @@
+repos:
+        - repo: https://github.com/pre-commit/pre-commit-hooks
+          rev: v4.3.0
+          hooks:
+                - id: check-yaml
+                - id: end-of-file-fixer
+                  exclude: \.txt$
+                - id: trailing-whitespace  # Remove trailing whitespaces
+                - id: check-merge-conflict
+
+        # Fix common spelling mistakes
+        - repo: https://github.com/codespell-project/codespell
+          rev: v2.2.1
+          hooks:
+                - id: codespell
+                  args: [
+                    '--ignore-words-list', 'nd,alos',
+                    '--ignore-regex', '\bhist\b',
+                    '--'
+                  ]
+                  types_or: [python, rst, markdown]
+                  files: ^(xdem|docs|tests)/
+
+        # Replace relative imports (e.g. 'from . import georaster' -> 'from geoutils import georaster')
+        - repo: https://github.com/MarcoGorelli/absolufy-imports
+          rev: v0.3.1
+          hooks:
+                - id: absolufy-imports
+
+        # Format the code aggressively using black
+        - repo: https://github.com/psf/black
+          rev: 22.8.0
+          hooks:
+                  - id: black
+                    args: [--line-length=120]
+
+        # Lint the code using flake8
+        - repo: https://gitlab.com/pycqa/flake8
+          rev: 3.9.2
+          hooks:
+                - id: flake8
+                  # More than one argument in the second list, so need to pass arguments as below (and -- to finish)
+                  args: [
+                          '--max-line-length', '120',  # we can write dicts however we want
+                          '--extend-ignore', 'E203,C408', # flake8 disagrees with black, so this should be ignored.
+                          '--'
+                  ]
+                  additional_dependencies:
+                          - flake8-comprehensions==3.1.0
+                          - flake8-bugbear==21.3.2
+                  files: ^(xdem|tests)
+
+        # Lint the code using mypy
+        - repo: https://github.com/pre-commit/mirrors-mypy
+          rev: v0.971
+          hooks:
+                - id: mypy
+                  args: [
+                        --config-file=mypy.ini,
+                        --strict,
+                        --implicit-optional,
+                        --ignore-missing-imports,  # Don't warn about stubs since pre-commit runs in a limited env
+                        --allow-untyped-calls,  # Dynamic function/method calls are okay. Untyped function definitions are not okay.
+                        --show-error-codes,
+                        --no-warn-unused-ignores,  # Ignore 'type: ignore' comments that are not used.
+                        --disable-error-code=attr-defined,  # "Module has no attribute 'XXX'" occurs because of the pre-commit env.
+                        --disable-error-code=name-defined,  # "Name 'XXX' is not defined" occurs because of the pre-commit env.
+                        --disable-error-code=var-annotated,
+                        --disable-error-code=no-any-return
+
+                  ]
+                  additional_dependencies: [tokenize-rt==3.2.0, numpy==1.22]
+                  files: ^(xdem|tests|docs/code)
+
+
+        # Sort imports using isort
+        - repo: https://github.com/PyCQA/isort
+          rev: 5.10.1
+          hooks:
+                  - id: isort
+                    args: ["--profile", "black"]
+
+        # Automatically upgrade syntax to a minimum version
+        - repo: https://github.com/asottile/pyupgrade
+          rev: v2.38.0
+          hooks:
+                - id: pyupgrade
+                  args: [--py37-plus]
+
+        # Various formattings
+        - repo: https://github.com/pre-commit/pygrep-hooks
+          rev: v1.9.0
+          hooks:
+                # Single backticks should apparently not be used
+                - id: rst-backticks
+                # Check that all directives end with double colon
+                - id: rst-directive-colons
+                  types: [text]
+                  types_or: [python, rst]
+                # Inline code should not touch normal text
+                - id: rst-inline-touching-normal
+                  types: [text]
+                  types_or: [python, rst]
+                # Eval should never be used (can do arbitrary code execution)
+                - id: python-no-eval
+                # Enforce the use of type annotations instead of docstring type comments
+                - id: python-use-type-annotations
+
+        # Add custom regex lints (see .relint.yml)
+        - repo: https://github.com/codingjoe/relint
+          rev: 1.4.0
+          hooks:
+                - id: relint
diff --git a/.relint.yml b/.relint.yml
@@ -0,0 +1,3 @@
+- name: Type hint in docstring
+  pattern: ':[r]?type '
+  filePattern: .*\.py
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,13 +1,25 @@
 # How to contribute
 
+## Overview: making a contribution
+For more details, see the rest of this document.
+
+1. Fork and clone the repository.
+2. Set up the development environment.
+3. Create a branch for the new feature or bug fix.
+4. Make your changes, ensuring they are conventions-compliant.
+5. Commit, making sure to run `pre-commit` separately if not installed as git hook.
+6. Push.
+7. Open a Pull Request to discuss and eventually merge.
+8. You can now delete your feature branch.
+
 ## Rights
 The MIT license (see LICENSE) applies to all contributions.
 
 ## Issue Conventions
 When submitting bugs, please include the following information:
 * Operating system type and version (Windows 10 / Ubuntu 18.04 etc.).
 * The version and source of `xdem` (PyPi, Anaconda, GitHub or elsewhere?).
-* The version and source of `geoutils`, `rasterio` and `GDAL`. 
+* The version and source of `geoutils`, `rasterio` and `GDAL`.
 
 Please search existing issues, open and closed, before creating a new one.
 
@@ -76,3 +88,17 @@ It is also recommended to try the tests from the parent directory, to validate t
 cd ../  # Change to the parent directory
 pytest xdem
 ```
+
+### Formatting and linting
+To merge a PR in xdem, the code has to adhere to the standards set in place.
+We use a number of tools to validate contributions, triggered using `pre-commit` (see `.pre-commit-config.yaml` for the exact tools).
+
+`pre-commit` is made to be installed as a "pre-commit hook" for git, so the checks have to pass before committing. Before committing for the first time, you need to install the hook:
+```bash
+pre-commit install
+```
+
+Pre-commit can also be run as a separate tool:
+```bash
+pre-commit run --all-files
+```
diff --git a/HOW_TO_RELEASE.md b/HOW_TO_RELEASE.md
@@ -15,7 +15,7 @@ An automatic GitHub action will start to push and publish the new release to PyP
 
 - PyPI will block the upload, so the GitHub action failed. All is fine.
 - You can now edit the version number on the main branch.
-- Before releasing, you need to delete **both** the tag and the release of the previous release. If you release with the same tag without deletion, it will ignore your commit changing the version number, and PyPI will block the upload again. You're stuck in a circle. 
+- Before releasing, you need to delete **both** the tag and the release of the previous release. If you release with the same tag without deletion, it will ignore your commit changing the version number, and PyPI will block the upload again. You're stuck in a circle.
 
 ## The hard way
 

diff --git a/README.md b/README.md
@@ -28,14 +28,14 @@ conda install mamba -n base -c conda-forge
 
 Once installed, the same commands can be run by simply replacing `conda` by `mamba`. More details available through the [mamba project](https://github.com/mamba-org/mamba).
 
-If running into the `sklearn` error `ImportError: dlopen: cannot load any more object with static TLS`, your system 
+If running into the `sklearn` error `ImportError: dlopen: cannot load any more object with static TLS`, your system
 needs to update its `glibc` (see details [here](https://github.com/scikit-learn/scikit-learn/issues/14485#issuecomment-822678559)).
-If you have no administrator right on the system, you might be able to circumvent this issue by installing a working 
+If you have no administrator right on the system, you might be able to circumvent this issue by installing a working
 environment with specific downgraded versions of `scikit-learn` and `numpy`:
 ```bash
 conda create -n xdem-env -c conda-forge xdem scikit-learn==0.20.3 numpy==1.19.*
 ```
-On very old systems, if the above install results in segmentation faults, try setting more specifically 
+On very old systems, if the above install results in segmentation faults, try setting more specifically
 `numpy==1.19.2=py37h54aff64_0` (worked with Debian 8.11, GLIBC 2.19).
 
 ### Installing with pip
@@ -59,7 +59,7 @@ After installing, we recommend to check that everything is working by running th
 $ pytest -rA
 ```
 
-## Structure 
+## Structure
 
 xdem are for now composed of three libraries:
 - `coreg.py` with tools covering differet aspects of DEM coregistration
@@ -121,4 +121,3 @@ difference.save("path/to/difference.tif")
 By default, `second_dem` is reprojected to fit `first_dem`.
 This can be switched with the keyword argument `reference="second"`.
 The resampling method can also be changed (e.g. `resampling_method="nearest"`) from the default `"cubic_spline"`.
-
diff --git a/dev-environment.yml b/dev-environment.yml
@@ -34,4 +34,3 @@ dependencies:
 
   - pip:
     - git+https://github.com/GlacioHack/geoutils.git
-
diff --git a/docs/source/_templates/module.rst b/docs/source/_templates/module.rst
@@ -3,7 +3,7 @@
 
 .. automodule:: {{ fullname }}
 
-   .. contents:: Contents 
+   .. contents:: Contents
       :local:
 
    {% block functions %}

diff --git a/docs/source/about_xdem.rst b/docs/source/about_xdem.rst
@@ -5,13 +5,12 @@ About xDEM
 
 xDEM is a set of open-source Python tools to facilitate the postprocessing of DEMs, and more general with georeferenced rasters. It is designed by geoscientists for geoscientists, although currently our group has a strong focus on glaciological applications.
 
-We are not software developers, but we try our best to offer tools that can be useful to a larger group, are well documented and are reliable and maintained. All develpment and maintenance is made on a voluntary basis and we welcome any new contributors. See some information on how to contribute in the dedicated page of our `GitHub repository <https://github.com/GlacioHack/xdem/blob/main/CONTRIBUTING.md>`_.
+We are not software developers, but we try our best to offer tools that can be useful to a larger group, are well documented and are reliable and maintained. All development and maintenance is made on a voluntary basis and we welcome any new contributors. See some information on how to contribute in the dedicated page of our `GitHub repository <https://github.com/GlacioHack/xdem/blob/main/CONTRIBUTING.md>`_.
 
 The core concepts behind *xdem* are:
 
-**Ease of use**: Most operations require only a few lines of code. The module was developed with a focus on remote sensing and geoscience applications. 
+**Ease of use**: Most operations require only a few lines of code. The module was developed with a focus on remote sensing and geoscience applications.
 
 **Modularity**: We offer a set of options, rather than favoring a single method. Everyone should be able to contribute to xdem and add to its functionalities.
 
 **Reproducibility**: Version-controlled, releases saved with DOI and test-based development ensure our code always performs as expected.
-
diff --git a/docs/source/api.rst b/docs/source/api.rst
@@ -20,9 +20,9 @@ Full information about xdem's functionality is provided on this page.
 
 .. automodule:: xdem
 
-   .. contents:: Contents 
+   .. contents:: Contents
       :local:
-        
+
    DEM
    ---
    .. autoclass:: DEM
@@ -34,7 +34,7 @@ Full information about xdem's functionality is provided on this page.
 
    .. minigallery:: xdem.DEM
         :add-heading:
-        
+
    DEMCollection
    -------------
    .. autoclass:: DEMCollection

diff --git a/docs/source/biascorr.rst b/docs/source/biascorr.rst
@@ -13,4 +13,4 @@ TODO: In construction
 Terrain biases
 --------------
 
-TODO: In construction
+TODO: In construction
diff --git a/docs/source/code/comparison.py b/docs/source/code/comparison.py
@@ -9,7 +9,6 @@
 
 import xdem
 
-
 # Load a reference DEM from 2009
 dem_2009 = xdem.DEM(xdem.examples.get_path("longyearbyen_ref_dem"), datetime=datetime(2009, 8, 1))
 # Load a DEM from 1990
@@ -48,11 +47,7 @@
 # SECTION: dDEM interpolation
 #############################
 
-ddem = xdem.dDEM(
-    raster=dem_2009 - dem_1990,
-    start_time=dem_1990.datetime,
-    end_time=dem_2009.datetime
-)
+ddem = xdem.dDEM(raster=dem_2009 - dem_1990, start_time=dem_1990.datetime, end_time=dem_2009.datetime)
 
 # The example DEMs are void-free, so let's make some random voids.
 # Introduce 50000 nans randomly throughout the dDEM.

diff --git a/docs/source/code/comparison_plot_local_hypsometric_interpolation.py b/docs/source/code/comparison_plot_local_hypsometric_interpolation.py
@@ -9,13 +9,9 @@
 dem_1990 = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem"))
 outlines_1990 = gu.Vector(xdem.examples.get_path("longyearbyen_glacier_outlines"))
 
-ddem = xdem.dDEM(
-    dem_2009 - dem_1990,
-    start_time=np.datetime64("1990-08-01"),
-    end_time=np.datetime64("2009-08-01")
-)
+ddem = xdem.dDEM(dem_2009 - dem_1990, start_time=np.datetime64("1990-08-01"), end_time=np.datetime64("2009-08-01"))
 
-ddem.data /= (2009 - 1990)
+ddem.data /= 2009 - 1990
 
 scott_1990 = outlines_1990.query("NAME == 'Scott Turnerbreen'")
 mask = scott_1990.create_mask(ddem)

diff --git a/docs/source/code/comparison_plot_regional_hypsometric_interpolation.py b/docs/source/code/comparison_plot_regional_hypsometric_interpolation.py
@@ -9,13 +9,9 @@
 dem_1990 = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem"))
 outlines_1990 = gu.Vector(xdem.examples.get_path("longyearbyen_glacier_outlines"))
 
-ddem = xdem.dDEM(
-    dem_2009 - dem_1990,
-    start_time=np.datetime64("1990-08-01"),
-    end_time=np.datetime64("2009-08-01")
-)
+ddem = xdem.dDEM(dem_2009 - dem_1990, start_time=np.datetime64("1990-08-01"), end_time=np.datetime64("2009-08-01"))
 
-ddem.data /= (2009 - 1990)
+ddem.data /= 2009 - 1990
 
 mask = outlines_1990.create_mask(ddem)
 

diff --git a/docs/source/code/comparison_plot_spatial_interpolation.py b/docs/source/code/comparison_plot_spatial_interpolation.py
@@ -9,11 +9,7 @@
 dem_1990 = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem"))
 outlines_1990 = gu.Vector(xdem.examples.get_path("longyearbyen_glacier_outlines"))
 
-ddem = xdem.dDEM(
-    dem_2009 - dem_1990,
-    start_time=np.datetime64("1990-08-01"),
-    end_time=np.datetime64("2009-08-01")
-)
+ddem = xdem.dDEM(dem_2009 - dem_1990, start_time=np.datetime64("1990-08-01"), end_time=np.datetime64("2009-08-01"))
 # The example DEMs are void-free, so let's make some random voids.
 ddem.data.mask = np.zeros_like(ddem.data, dtype=bool)  # Reset the mask
 # Introduce 50000 nans randomly throughout the dDEM.

diff --git a/docs/source/code/comparison_print_cumulative_dh.py b/docs/source/code/comparison_print_cumulative_dh.py
@@ -2,6 +2,7 @@
 import contextlib
 import io
 import sys
+
 import xdem
 
 sys.path.insert(0, "code/")  # The base directory is source/, so to find comparison.py, it has to be source/code/
@@ -13,7 +14,7 @@
 dems = xdem.DEMCollection(
     [comparison.dem_1990, comparison.dem_2009, comparison.dem_2060],
     outlines=comparison.outlines,
-    reference_dem=comparison.dem_2009
+    reference_dem=comparison.dem_2009,
 )
 
 dems.subtract_dems()

diff --git a/docs/source/code/coregistration.py b/docs/source/code/coregistration.py
@@ -8,7 +8,6 @@
 import xdem
 from xdem import coreg
 
-
 # Load the data using xdem and geoutils (could be with rasterio and geopandas instead)
 # Load a reference DEM from 2009
 ref_dem = xdem.DEM(xdem.examples.get_path("longyearbyen_ref_dem"))