Add numpydoc Sphinx extension (stfc#44)

* Fix docstrings for numpy style * Add numpydoc Sphinx extension * Add numpydoc pre-commit * Fix old docstrings for numpydoc * Fix typos * Update janus_core/mlip_calculators.py Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com> * Update janus_core/single_point.py Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com> * Change default device to cpu --------- Co-authored-by: ElliottKasoar <ElliottKasoar@users.noreply.github.com> Co-authored-by: Alin Marin Elena <alin@elena.re> Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com>
ElliottKasoar · Mar 1, 2024 · 863497b · 863497b
1 parent a100b69
commit 863497b
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 42 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -54,11 +54,10 @@ jobs:
     steps:
     - uses: actions/checkout@v4
 
-
     - name: Set up Python 3.11
       uses: actions/setup-python@v5
       with:
-        python-version: 3.11
+        python-version: '3.11'
 
     - name: Install poetry
       run: pipx install poetry
@@ -80,7 +79,7 @@ jobs:
     - name: Set up Python 3.11
       uses: actions/setup-python@v5
       with:
-        python-version: 3.11
+        python-version: '3.11'
 
     - name: Install poetry
       run: pipx install poetry

diff --git a/.github/workflows/publish-on-pypi.yml b/.github/workflows/publish-on-pypi.yml
@@ -18,7 +18,7 @@ jobs:
     - name: Set up Python 3.11
       uses: actions/setup-python@v5
       with:
-        python-version: 3.11
+        python-version: '3.11'
 
     - name: Install poetry
       run: pipx install poetry

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -37,3 +37,9 @@ repos:
           docs/.*|
       )$
     entry: pylint
+
+- repo: https://github.com/numpy/numpydoc
+  rev: v1.6.0
+  hooks:
+    - id: numpydoc-validation
+      files: ^janus_core/
diff --git a/docs/source/conf.py b/docs/source/conf.py
@@ -26,13 +26,17 @@
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
 extensions = [
+    "numpydoc",
     "sphinx.ext.autodoc",
-    "sphinx.ext.mathjax",
     "sphinx.ext.intersphinx",
+    "sphinx.ext.mathjax",
     "sphinx.ext.viewcode",
     "sphinxcontrib.contentui",
 ]
 
+numpydoc_validation_checks = {"all", "EX01", "SA01", "ES01"}
+numpydoc_validation_exclude = {r"\.__weakref__$", r"\.__repr__$"}
+
 intersphinx_mapping = {
     "python": ("https://docs.python.org/3", None),
     "numpy": ("https://numpy.org/doc/stable/", None),

diff --git a/janus_core/__init__.py b/janus_core/__init__.py
@@ -1,6 +1,3 @@
-"""janus_core.
-
-Tools for machine learnt interatomic potentials.
-"""
+"""Tools for machine learnt interatomic potentials."""
 
 __version__ = "0.1.0a1"
diff --git a/janus_core/geom_opt.py b/janus_core/geom_opt.py
@@ -24,7 +24,8 @@ def optimize(
     struct_kwargs: Optional[dict[str, Any]] = None,
     traj_kwargs: Optional[dict[str, Any]] = None,
 ) -> Atoms:
-    """Optimize geometry of input structure.
+    """
+    Optimize geometry of input structure.
 
     Parameters
     ----------
@@ -33,21 +34,21 @@ def optimize(
     fmax : float
         Set force convergence criteria for optimizer in units eV/Å.
     dyn_kwargs : Optional[dict[str, Any]]
-        kwargs to pass to dyn.run. Default is {}.
+        Keyword arguments to pass to dyn.run. Default is {}.
     filter_func : Optional[callable]
         Apply constraints to atoms through ASE filter function.
         Default is `FrechetCellFilter` if available otherwise `ExpCellFilter`.
     filter_kwargs : Optional[dict[str, Any]]
-        kwargs to pass to filter_func. Default is {}.
-    optimzer : callable
+        Keyword arguments to pass to filter_func. Default is {}.
+    optimizer : callable
         ASE optimization function. Default is `LBFGS`.
     opt_kwargs : Optional[dict[str, Any]]
-        kwargs to pass to optimzer. Default is {}.
+        Keyword arguments to pass to optimizer. Default is {}.
     struct_kwargs : Optional[dict[str, Any]]
-        kwargs to pass to ase.io.write to save optimized structure.
+        Keyword arguments to pass to ase.io.write to save optimized structure.
         Must include "filename" keyword. Default is {}.
     traj_kwargs : Optional[dict[str, Any]]
-        kwargs to pass to ase.io.write to save optimization trajectory.
+        Keyword arguments to pass to ase.io.write to save optimization trajectory.
         Must include "filename" keyword. Default is {}.
 
     Returns

diff --git a/janus_core/mlip_calculators.py b/janus_core/mlip_calculators.py
@@ -1,6 +1,7 @@
-"""Configure MLIP calculators.
+"""
+Configure MLIP calculators.
 
-Similar in spirit with matcalc and quacc approaches
+Similar in spirit to matcalc and quacc approaches
 - https://github.com/materialsvirtuallab/matcalc
 - https://github.com/Quantum-Accelerators/quacc.git
 """
@@ -15,27 +16,32 @@
 
 def choose_calculator(
     architecture: Literal[architectures] = "mace",
-    device: Literal[devices] = "cuda",
+    device: Literal[devices] = "cpu",
     **kwargs,
 ) -> Calculator:
-    """Choose MLIP calculator to configure.
+    """
+    Choose MLIP calculator to configure.
 
     Parameters
     ----------
     architecture : Literal[architectures], optional
         MLIP architecture. Default is "mace".
+    device : Literal[devices]
+        Device to run calculator on. Default is "cpu".
+    **kwargs
+        Additional keyword arguments passed to the selected calculator.
+
+    Returns
+    -------
+    Calculator
+        Configured MLIP calculator.
 
     Raises
     ------
     ModuleNotFoundError
         MLIP module not correctly been installed.
     ValueError
         Invalid architecture specified.
-
-    Returns
-    -------
-    calculator : Calculator
-        Configured MLIP calculator.
     """
     # pylint: disable=import-outside-toplevel, too-many-branches, import-error
     # Optional imports handled via `architecture`. We could catch these,

diff --git a/janus_core/single_point.py b/janus_core/single_point.py
@@ -1,4 +1,4 @@
-"""Perpare and perform single point calculations."""
+"""Prepare and perform single point calculations."""
 
 import pathlib
 from typing import Any, Literal, Optional, Union
@@ -10,7 +10,41 @@
 
 
 class SinglePoint:
-    """Perpare and perform single point calculations."""
+    """
+    Prepare and perform single point calculations.
+
+    Parameters
+    ----------
+    system : str
+        System to simulate.
+    architecture : Literal[architectures]
+        MLIP architecture to use for single point calculations.
+        Default is "mace_mp".
+    device : Literal[devices]
+        Device to run model on. Default is "cpu".
+    read_kwargs : Optional[dict[str, Any]]
+        Keyword arguments to pass to ase.io.read. Default is {}.
+    **kwargs
+        Additional keyword arguments passed to the selected calculator.
+
+    Attributes
+    ----------
+    architecture : Literal[architectures]
+        MLIP architecture to use for single point calculations.
+    system : str
+        System to simulate.
+    device : Literal[devices]
+        Device to run MLIP model on.
+
+    Methods
+    -------
+    read_system(**kwargs)
+        Read system and system name.
+    set_calculator(**kwargs)
+        Configure calculator and attach to system.
+    run_single_point(properties=None)
+        Run single point calculations.
+    """
 
     def __init__(
         self,
@@ -21,19 +55,21 @@ def __init__(
         **kwargs,
     ) -> None:
         """
-        Initialise class.
+        Read the system being simulated and attach an MLIP calculator.
 
-        Attributes
+        Parameters
         ----------
         system : str
             System to simulate.
         architecture : Literal[architectures]
             MLIP architecture to use for single point calculations.
             Default is "mace_mp".
         device : Literal[devices]
-            Device to run model on. Default is "cpu".
+            Device to run MLIP model on. Default is "cpu".
         read_kwargs : Optional[dict[str, Any]]
-            kwargs to pass to ase.io.read. Default is {}.
+            Keyword arguments to pass to ase.io.read. Default is {}.
+        **kwargs
+            Additional keyword arguments passed to the selected calculator.
         """
         self.architecture = architecture
         self.device = device
@@ -45,23 +81,32 @@ def __init__(
         self.set_calculator(**kwargs)
 
     def read_system(self, **kwargs) -> None:
-        """Read system and system name.
+        """
+        Read system and system name.
 
         If the file contains multiple structures, only the last configuration
         will be read by default.
+
+        Parameters
+        ----------
+        **kwargs
+            Keyword arguments passed to ase.io.read.
         """
         self.sys = read(self.system, **kwargs)
         self.sysname = pathlib.Path(self.system).stem
 
     def set_calculator(
         self, read_kwargs: Optional[dict[str, Any]] = None, **kwargs
     ) -> None:
-        """Configure calculator and attach to system.
+        """
+        Configure calculator and attach to system.
 
         Parameters
         ----------
         read_kwargs : Optional[dict[str, Any]]
-            kwargs to pass to ase.io.read. Default is {}.
+            Keyword arguments to pass to ase.io.read. Default is {}.
+        **kwargs
+            Additional keyword arguments passed to the selected calculator.
         """
         calculator = choose_calculator(
             architecture=self.architecture,
@@ -79,11 +124,12 @@ def set_calculator(
             self.sys.calc = calculator
 
     def _get_potential_energy(self) -> Union[float, list[float]]:
-        """Calculate potential energy using MLIP.
+        """
+        Calculate potential energy using MLIP.
 
         Returns
         -------
-        potential_energy : Union[float, list[float]]
+        Union[float, list[float]]
             Potential energy of system(s).
         """
         if isinstance(self.sys, list):
@@ -92,11 +138,12 @@ def _get_potential_energy(self) -> Union[float, list[float]]:
         return self.sys.get_potential_energy()
 
     def _get_forces(self) -> Union[ndarray, list[ndarray]]:
-        """Calculate forces using MLIP.
+        """
+        Calculate forces using MLIP.
 
         Returns
         -------
-        forces : Union[ndarray, list[ndarray]]
+        Union[ndarray, list[ndarray]]
             Forces of system(s).
         """
         if isinstance(self.sys, list):
@@ -105,11 +152,12 @@ def _get_forces(self) -> Union[ndarray, list[ndarray]]:
         return self.sys.get_forces()
 
     def _get_stress(self) -> Union[ndarray, list[ndarray]]:
-        """Calculate stress using MLIP.
+        """
+        Calculate stress using MLIP.
 
         Returns
         -------
-        stress : Union[ndarray, list[ndarray]]
+        Union[ndarray, list[ndarray]]
             Stress of system(s).
         """
         if isinstance(self.sys, list):
@@ -120,7 +168,8 @@ def _get_stress(self) -> Union[ndarray, list[ndarray]]:
     def run_single_point(
         self, properties: Optional[Union[str, list[str]]] = None
     ) -> dict[str, Any]:
-        """Run single point calculations.
+        """
+        Run single point calculations.
 
         Parameters
         ----------
@@ -130,7 +179,7 @@ def run_single_point(
 
         Returns
         -------
-        results : dict[str, Any]
+        dict[str, Any]
             Dictionary of calculated results.
         """
         results = {}

diff --git a/pyproject.toml b/pyproject.toml
@@ -47,6 +47,7 @@ optional = true
 [tool.poetry.group.docs.dependencies]
 furo = "^2024.1.29"
 markupsafe = "<2.1"
+numpydoc = "^1.6.0"
 sphinx = "^7.2.6"
 sphinxcontrib-contentui = "^0.2.5"
 sphinxcontrib-details-directive = "^0.1"
@@ -84,3 +85,17 @@ source=["janus_core"]
 line_length = 120
 force_sort_within_sections = true
 sections = ['FUTURE', 'STDLIB', 'THIRDPARTY', 'FIRSTPARTY', 'LOCALFOLDER']
+
+[tool.numpydoc_validation]
+# report on all checks, except the below
+checks = [
+    "all",
+    "EX01",
+    "SA01",
+    "ES01",
+]
+# Don't report on objects that match any of these regex
+exclude = [
+    ".__weakref__$",
+    ".__repr__$",
+]