Type checking (duartegroup#277)

* Resolve conflicts with v1.4.0 * Start removal of dep functions [skip actions] * Add force constant [skip actions] * NEB refactoring * Fix units for div mul * Update changelog * Simplify and add comment * Remove more deprecated functions [skip actions] * Fix tests * Ignore type checking in tests + more tests * More typing * More typing and update lint workflow * Add some tests * Typing and benchmark script exclusion * Add test * Pin in requirements file and use mamba --------- Co-authored-by: Shoubhik Maiti <[email protected]>
aleksy-k · Jul 3, 2023 · f9fb7a0 · f9fb7a0
1 parent e05752d
commit f9fb7a0
Show file tree

Hide file tree

Showing 142 changed files with 1,989 additions and 1,154 deletions.
diff --git a/.github/workflows/catch.yml b/.github/workflows/catch.yml
@@ -21,7 +21,7 @@ jobs:
         os: [ubuntu-latest, macOS-latest]
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Create Build Environment
         run: mkdir $GITHUB_WORKSPACE/autode/ext/build

diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
@@ -13,7 +13,7 @@ jobs:
         shell: bash -l {0}
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v3
 
     - uses: conda-incubator/setup-miniconda@v2
       with:
@@ -32,7 +32,7 @@ jobs:
       run: |
         cd doc
         make html
-        touch _build/html/.nojekyll 
+        touch _build/html/.nojekyll
 
     - name: Deploy
       uses: JamesIves/[email protected]

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -14,11 +14,10 @@ jobs:
       - name: Checkout repository
         uses: actions/checkout@v3
 
-      - name: Black
-        uses: psf/black@stable
+      - name: Run pre-commit
+        uses: pre-commit/[email protected]
         with:
-          options: "--check --verbose"
-          version: "~= 22.0"
+          extra_args: --all-files
 
       - name: Initialize CodeQL
         uses: github/codeql-action/init@v2

diff --git a/.github/workflows/pytest.yml b/.github/workflows/pytest.yml
@@ -17,31 +17,33 @@ jobs:
   test:
     name: Env (${{ matrix.python-version }}, ${{ matrix.os }})
     runs-on: ${{ matrix.os }}
-    
+
     strategy:
       fail-fast: true
       matrix:
         os: ["ubuntu-latest", "macos-latest", "windows-latest"]
         python-version: ["3.8", "3.9", "3.10"]
-      
+
     defaults:
       run:
         shell: bash -l {0}
-        
+
     steps:
-    - uses: actions/checkout@v2
-        
+    - uses: actions/checkout@v3
+
     - uses: conda-incubator/setup-miniconda@v2
       with:
         python-version: ${{ matrix.python-version }}
-        channels: conda-forge,defaults
-        auto-update-conda: true
+        activate-environment: test
+        mamba-version: "*"
+        channels: conda-forge
 
     - name: Install
       run: |
-        conda activate test
-        conda install --file requirements.txt
-        conda install --file tests/requirements.txt
+        mamba install \
+          --file requirements.txt \
+          --file tests/requirements.txt \
+          --channel conda-forge
         python -m pip install . --no-deps
 
     - name: Test

diff --git a/.github/workflows/pytest_cov.yml b/.github/workflows/pytest_cov.yml
@@ -28,21 +28,23 @@ jobs:
     defaults:
       run:
         shell: bash -l {0}
-        
+
     steps:
-    - uses: actions/checkout@v2
-        
+    - uses: actions/checkout@v3
+
     - uses: conda-incubator/setup-miniconda@v2
       with:
         python-version: ${{ matrix.python-version }}
-        channels: conda-forge,defaults
-        auto-update-conda: true
+        activate-environment: test
+        mamba-version: "*"
+        channels: conda-forge
 
     - name: Install
       run: |
-        conda activate test
-        conda install --file requirements.txt
-        conda install --file tests/requirements.txt
+        mamba install \
+          --file requirements.txt \
+          --file tests/requirements.txt \
+          --channel conda-forge
         source .github/scripts/install_xtb_ci.sh
         python -m pip install . --no-deps
 

diff --git a/.gitignore b/.gitignore
@@ -21,3 +21,4 @@ tests/*.xyz
 *.pyc
 .autode_calculations
 tests/initial_path/*
+.mypy_cache
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,6 +1,19 @@
 repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v3.2.0
+    hooks:
+      - id: trailing-whitespace
+      - id: mixed-line-ending
+
   - repo: https://github.com/psf/black
     rev: 22.10.0
     hooks:
       - id: black
-        language_version: python3.9
+        language_version: python3
+
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v1.0.1
+    hooks:
+    - id: mypy
+      exclude: "tests/|doc/|examples/"
+      args: [--ignore-missing-imports]
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,5 +1,5 @@
 # Contributing to autodE
 
 Contributions in any form are very much welcome. To make managing these
-easier, we kindly ask that you follow the guidelines outlined 
+easier, we kindly ask that you follow the guidelines outlined
 [here](https://duartegroup.github.io/autodE/dev/contributing.html).
diff --git a/LICENSE.md b/LICENSE.md
@@ -1,7 +1,7 @@
 
 The MIT License (MIT)
 
-Copyright (c) 2018 
+Copyright (c) 2018
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

diff --git a/README.md b/README.md
@@ -4,7 +4,7 @@
 ***
 ## Introduction
 
-**autodE** is a Python module initially designed for the automated calculation of reaction profiles from SMILES strings of 
+**autodE** is a Python module initially designed for the automated calculation of reaction profiles from SMILES strings of
 reactant(s) and product(s). Current features include: transition state location, conformer searching, atom mapping,
 Python wrappers for a range of electronic structure theory codes, SMILES parsing, association complex generation, and
  reaction profile generation.
@@ -34,8 +34,8 @@ see the [installation guide](https://duartegroup.github.io/autodE/install.html)
 
 ## Usage
 
-Reaction profiles in  **autodE** are generated by initialising _Reactant_ and _Product_ objects, 
-generating a _Reaction_ from those and invoking _calculate_reaction_profile()_. 
+Reaction profiles in  **autodE** are generated by initialising _Reactant_ and _Product_ objects,
+generating a _Reaction_ from those and invoking _calculate_reaction_profile()_.
 For example, to  calculate the profile for a 1,2 hydrogen shift in a propyl radical:
 
 ```python
@@ -56,19 +56,19 @@ additional documentation.
 
 ## Development
 
-There is a [slack workspace](https://autodeworkspace.slack.com) for development and discussion - please 
-[email](mailto:[email protected]?subject=autodE%20slack) to be added. Pull requests are 
+There is a [slack workspace](https://autodeworkspace.slack.com) for development and discussion - please
+[email](mailto:[email protected]?subject=autodE%20slack) to be added. Pull requests are
 very welcome but must pass all the unit tests prior to being merged. Please write code and tests!
-See the [todo list](https://github.com/duartegroup/autodE/projects/1) for features on the horizon. 
-Bugs and feature requests should be raised on the [issue page](https://github.com/duartegroup/autodE/issues). 
+See the [todo list](https://github.com/duartegroup/autodE/projects/1) for features on the horizon.
+Bugs and feature requests should be raised on the [issue page](https://github.com/duartegroup/autodE/issues).
 
-> **_NOTE:_**  We'd love more contributors to this project! 
+> **_NOTE:_**  We'd love more contributors to this project!
 
 
 ## Citation
 
 If **autodE** is used in a publication please consider citing the [paper](https://doi.org/10.1002/anie.202011941):
- 
+
 ```
 @article{autodE,
   doi = {10.1002/anie.202011941},

diff --git a/autode/atoms.py b/autode/atoms.py
@@ -17,9 +17,9 @@ class Atom:
     def __init__(
         self,
         atomic_symbol: str,
-        x: float = 0.0,
-        y: float = 0.0,
-        z: float = 0.0,
+        x: Any = 0.0,
+        y: Any = 0.0,
+        z: Any = 0.0,
         atom_class: Optional[int] = None,
         partial_charge: Optional[float] = None,
     ):
@@ -876,11 +876,11 @@ def are_planar(self, distance_tol: Distance = Distance(1e-3, "Å")) -> bool:
         arr = self.coordinates.to("Å")
 
         if isinstance(distance_tol, Distance):
-            distance_tol = float(distance_tol.to("Å"))
+            distance_tol_float = float(distance_tol.to("Å"))
 
         else:
             logger.warning("Assuming a distance tolerance in units of Å")
-            distance_tol = float(distance_tol)
+            distance_tol_float = float(distance_tol)
 
         # Calculate a normal vector to the first two atomic vectors from atom 0
         x0 = arr[0, :]
@@ -890,7 +890,7 @@ def are_planar(self, distance_tol: Distance = Distance(1e-3, "Å")) -> bool:
 
             # Calculate the 0->i atomic vector, which must not have any
             # component in the direction in the normal if the atoms are planar
-            if np.dot(normal_vec, arr[i, :] - x0) > distance_tol:
+            if np.dot(normal_vec, arr[i, :] - x0) > distance_tol_float:
                 return False
 
         return True
@@ -930,7 +930,7 @@ def coordinates(self, value: np.ndarray):
             value (np.ndarray): Shape = (n_atoms, 3) or (3*n_atoms) as a
                                 row major vector
         """
-        if self.atoms is None:
+        if self._atoms is None:
             raise ValueError(
                 "Must have atoms set to be able to set the "
                 "coordinates of them"
@@ -984,12 +984,14 @@ def weight(self) -> Mass:
         if self.n_atoms == 0:
             return Mass(0.0)
 
-        return sum(atom.mass for atom in self.atoms)
+        return sum(atom.mass for atom in self.atoms)  # type: ignore
 
     def distance(self, i: int, j: int) -> Distance:
+        assert self.atoms is not None, "Must have atoms"
         return self.atoms.distance(i, j)
 
     def eqm_bond_distance(self, i: int, j: int) -> Distance:
+        assert self.atoms is not None, "Must have atoms"
         return self.atoms.eqm_bond_distance(i, j)
 
     def angle(self, i: int, j: int, k: int) -> Angle:
@@ -1024,6 +1026,8 @@ def angle(self, i: int, j: int, k: int) -> Angle:
         Raises:
             (ValueError): If any of the atom indexes are not present
         """
+        assert self.atoms is not None, "Must have atoms"
+
         if not self.atoms.idxs_are_present(i, j, k):
             raise ValueError(
                 f"Cannot calculate the angle between {i}-{j}-{k}."
@@ -1081,6 +1085,8 @@ def dihedral(self, w: int, x: int, y: int, z: int) -> Angle:
             (ValueError): If any of the atom indexes are not present in the
                           molecule
         """
+        assert self.atoms is not None, "Must have atoms"
+
         if not self.atoms.idxs_are_present(w, x, y, z):
             raise ValueError(
                 f"Cannot calculate the dihedral angle involving "
@@ -1107,8 +1113,8 @@ def dihedral(self, w: int, x: int, y: int, z: int) -> Angle:
             vec /= norm
 
         """
-        Dihedral angles are defined as from the IUPAC gold book: "the torsion 
-        angle between groups A and D is then considered to be positive if 
+        Dihedral angles are defined as from the IUPAC gold book: "the torsion
+        angle between groups A and D is then considered to be positive if
         the bond A-B is rotated in a clockwise direction through less than
         180 degrees"
         """
@@ -1638,7 +1644,7 @@ def transition_metals(cls, row: int):
 }
 
 """
-Although a π-bond may not be well defined, it is useful to have a notion of 
+Although a π-bond may not be well defined, it is useful to have a notion of
 a bond about which there is restricted rotation. The below sets are used to
 define which atoms may be π-bonded to another
 """

diff --git a/autode/bracket/base.py b/autode/bracket/base.py
@@ -68,11 +68,13 @@ def _name(self) -> str:
     @property
     def ts_guess(self) -> Optional["Species"]:
         """Get the TS guess from image-pair"""
+        assert self.imgpair is not None, "Must have an image pair for TS guess"
         return self.imgpair.ts_guess
 
     @property
     def converged(self) -> bool:
         """Whether the bracketing method has converged or not"""
+        assert self.imgpair is not None, "Must have an image pair"
 
         # NOTE: In bracketing methods, usually the geometry
         # optimisation is done in separate micro-iterations,
@@ -107,6 +109,8 @@ def _log_convergence(self) -> None:
         Log the convergence of the bracket method. Only logs macro-iters,
         subclasses may implement further logging for micro-iters
         """
+        assert self.imgpair is not None, "Must have an image pair to log"
+
         logger.info(
             f"{self._name} Macro-iteration #{self._macro_iter}: "
             f"Distance = {self.imgpair.dist:.4f}; Energy (initial species) = "
@@ -159,6 +163,8 @@ def _calculate(
         Actually runs the calculation, it is wrapped around in calculate()
         so that the results are placed in a sub-folder
         """
+        assert self.imgpair is not None, "Must have set image pair"
+
         n_cores = Config.n_cores if n_cores is None else int(n_cores)
         self.imgpair.set_method_and_n_cores(method, n_cores)
         self._initialise_run()
@@ -203,7 +209,8 @@ def _calculate(
 
         self.print_geometries()
         self.plot_energies()
-        self.ts_guess.print_xyz_file(filename=f"{self._name}_ts_guess.xyz")
+        if self.ts_guess is not None:
+            self.ts_guess.print_xyz_file(filename=f"{self._name}_ts_guess.xyz")
         return None
 
     def print_geometries(
@@ -218,6 +225,8 @@ def print_geometries(
         any CI-NEB run from the final end points. The default names for
         the trajectories must be set in individual subclasses
         """
+        assert self.imgpair is not None, "Must have an image pair to plot"
+
         init_trj_filename = (
             init_trj_filename
             if init_trj_filename is not None
@@ -263,6 +272,8 @@ def plot_energies(
             filename (str|None): Name of the file (optional)
             distance_metric (str): "relative" or "from_start" or "index"
         """
+        assert self.imgpair is not None, "Must have an image pair to plot"
+
         filename = (
             filename
             if filename is not None
@@ -279,6 +290,8 @@ def run_cineb(self) -> None:
         should bring the ends very close to the TS). The result from
         the CI-NEB calculation is stored as coordinates.
         """
+        assert self.imgpair is not None, "Must have image pair to run CINEB"
+
         if not self._micro_iter > 0:
             logger.error(
                 f"Must run {self._name} calculation before"