Merge pull request #108 from lsst/tickets/DM-42116

DM-42116: Add numpydoc validation

.github/workflows/docstyle.yaml

@@ -0,0 +1,28 @@
+name: Run docstyle
+
+on:
+  push:
+    branches:
+      - main
+  pull_request:
+
+jobs:
+  call-workflow:
+    uses: lsst/rubin_workflows/.github/workflows/docstyle.yaml@main
+    with:
+      args: "python/"
+  numpydoc:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Set up Python
+        uses: actions/setup-python@v4
+
+      - name: Install numpydoc
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install numpydoc
+
+      - name: Validate docstrings
+        run: python -m numpydoc.hooks.validate_docstrings $(find python -name "*.py")

.pre-commit-config.yaml

@@ -1,12 +1,12 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v4.5.0
     hooks:
       - id: check-yaml
       - id: end-of-file-fixer
       - id: trailing-whitespace
   - repo: https://github.com/psf/black
-    rev: 23.1.0
+    rev: 23.12.0
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python
@@ -15,11 +15,20 @@ repos:
         # https://pre-commit.com/#top_level-default_language_version
         language_version: python3.10
   - repo: https://github.com/pycqa/isort
-    rev: 5.12.0
+    rev: 5.13.2
     hooks:
       - id: isort
         name: isort (python)
   - repo: https://github.com/PyCQA/flake8
-    rev: 6.0.0
+    rev: 6.1.0
     hooks:
       - id: flake8
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    # Ruff version.
+    rev: v0.1.8
+    hooks:
+      - id: ruff
+  - repo: https://github.com/numpy/numpydoc
+    rev: "v1.6.0"
+    hooks:
+      - id: numpydoc-validation

pyproject.toml

@@ -157,3 +157,23 @@ max-doc-length = 79
 
 [tool.ruff.pydocstyle]
 convention = "numpy"
+
+[tool.numpydoc_validation]
+checks = [
+    "all",  # All except the rules listed below.
+    "SA01",  # See Also section.
+    "EX01",  # Example section.
+    "SS06",  # Summary can go into second line.
+    "GL01",  # Summary text can start on same line as """
+    "GL08",  # Do not require docstring.
+    "ES01",  # No extended summary required.
+    "RT01",  # Unfortunately our @property trigger this.
+    "RT02",  # Does not want named return value. DM style says we do.
+    "SS05",  # pydocstyle is better at finding infinitive verb.
+    "SA04",  # Do not require descriptions on all See Also entries.
+]
+exclude = [
+    '^__init__$',
+    '\._[a-zA-Z_]+$',  # Private methods.
+    "^test_.*",  # Do not test docstrings in test code.
+]

python/lsst/pex/config/comparison.py

@@ -118,9 +118,9 @@ def compareConfigs(name, c1, c2, shortcut=True, rtol=1e-8, atol=1e-8, output=Non
     name : `str`
         Name to use when reporting differences, typically created by
         `getComparisonName`.
-    v1 : `lsst.pex.config.Config`
+    c1 : `lsst.pex.config.Config`
         Left-hand side config to compare.
-    v2 : `lsst.pex.config.Config`
+    c2 : `lsst.pex.config.Config`
         Right-hand side config to compare.
     shortcut : `bool`, optional
         If `True`, return as soon as an inequality is found. Default is `True`.
@@ -163,7 +163,7 @@ def compareConfigs(name, c1, c2, shortcut=True, rtol=1e-8, atol=1e-8, output=Non
             if output is not None:
                 output("RHS is None for %s" % name)
             return False
-    if type(c1) != type(c2):
+    if type(c1) is not type(c2):
         if output is not None:
             output(f"Config types do not match for {name}: {type(c1)} != {type(c2)}")
         return False

python/lsst/pex/config/config.py

@@ -204,6 +204,15 @@ def _yaml_config_constructor(loader, node):
 class ConfigMeta(type):
     """A metaclass for `lsst.pex.config.Config`.
 
+    Parameters
+    ----------
+    name : `str`
+        Name to use for class.
+    bases : `~collections.abc.Iterable`
+        Base classes.
+    dict_ : `dict`
+        Additional parameters.
+
     Notes
     -----
     ``ConfigMeta`` adds a dictionary containing all `~lsst.pex.config.Field`
@@ -377,7 +386,6 @@ class Field(Generic[FieldTypeVar]):
     useful for annotating the type of the identifier (i.e. variable in previous
     example) and does nothing for assigning the dtype of the ``Field``.
 
-
     Examples
     --------
     Instances of ``Field`` should be used as class attributes of
@@ -436,7 +444,7 @@ def _parseTypingArgs(
 
         Raises
         ------
-        ValueError :
+        ValueError
             Raised if params is of incorrect length.
             Raised if a forward reference could not be resolved
             Raised if there is a conflict between params and values in kwds
@@ -528,7 +536,7 @@ def _setup(self, doc, dtype, default, check, optional, source, deprecated):
 
         self.source = source
         """The stack frame where this field is defined (`list` of
-        `lsst.pex.config.callStack.StackFrame`).
+        `~lsst.pex.config.callStack.StackFrame`).
         """
 
     def rename(self, instance):
@@ -755,7 +763,8 @@ def __set__(
             The config instance that contains this field.
         value : obj
             Value to set on this field.
-        at : `list` of `lsst.pex.config.callStack.StackFrame`
+        at : `list` of `~lsst.pex.config.callStack.StackFrame` or `None`,\
+                optional
             The call stack (created by
             `lsst.pex.config.callStack.getCallStack`).
         label : `str`, optional
@@ -898,6 +907,15 @@ def find_spec(self, fullname, path, target=None):
         """Find a module.
 
         Called as part of the ``import`` chain of events.
+
+        Parameters
+        ----------
+        fullname : `str`
+            Name of module.
+        path : `list` [`str`]
+            Search path. Unused.
+        target : `~typing.Any`, optional
+            Unused.
         """
         self._modules.add(fullname)
         # Return None because we don't do any importing.
@@ -1081,7 +1099,7 @@ def update(self, **kw):
 
         Parameters
         ----------
-        kw
+        **kw
             Keywords are configuration field names. Values are configuration
             field values.
 
@@ -1174,7 +1192,7 @@ def loadFromStream(self, stream, root="config", filename=None, extraLocals=None)
 
         Parameters
         ----------
-        stream : file-like object, `str`, `bytes`, or compiled string
+        stream : file-like object, `str`, `bytes`, or `~types.CodeType`
             Stream containing configuration override code.  If this is a
             code object, it should be compiled with ``mode="exec"``.
         root : `str`, optional
@@ -1221,7 +1239,7 @@ def loadFromString(self, code, root="config", filename=None, extraLocals=None):
 
         Parameters
         ----------
-        code : `str`, `bytes`, or compiled string
+        code : `str`, `bytes`, or `~types.CodeType`
             Stream containing configuration override code.
         root : `str`, optional
             Name of the variable in file that refers to the config being
@@ -1342,7 +1360,7 @@ def saveToStream(self, outfile, root="config", skipImports=False):
         outfile : file-like object
             Destination file object write the config into. Accepts strings not
             bytes.
-        root
+        root : `str`, optional
             Name to use for the root config variable. The same value must be
             used when loading (see `lsst.pex.config.Config.load`).
         skipImports : `bool`, optional
@@ -1521,7 +1539,7 @@ def formatHistory(self, name, **kwargs):
         ----------
         name : `str`
             Name of a `~lsst.pex.config.Field` in this config.
-        kwargs
+        **kwargs
             Keyword arguments passed to `lsst.pex.config.history.format`.
 
         Returns
@@ -1585,7 +1603,7 @@ def __delattr__(self, attr, at=None, label="deletion"):
             object.__delattr__(self, attr)
 
     def __eq__(self, other):
-        if type(other) == type(self):
+        if type(other) is type(self):
             for name in self._fields:
                 thisValue = getattr(self, name)
                 otherValue = getattr(other, name)
@@ -1735,15 +1753,15 @@ def _classFromPython(config_py):
     return pytype
 
 
-def unreduceConfig(cls, stream):
+def unreduceConfig(cls_, stream):
     """Create a `~lsst.pex.config.Config` from a stream.
 
     Parameters
     ----------
-    cls : `lsst.pex.config.Config`-type
+    cls_ : `lsst.pex.config.Config`-type
         A `lsst.pex.config.Config` type (not an instance) that is instantiated
         with configurations in the ``stream``.
-    stream : file-like object, `str`, or compiled string
+    stream : file-like object, `str`, or `~types.CodeType`
         Stream containing configuration override code.
 
     Returns
@@ -1755,6 +1773,6 @@ def unreduceConfig(cls, stream):
     --------
     lsst.pex.config.Config.loadFromStream
     """
-    config = cls()
+    config = cls_()
     config.loadFromStream(stream)
     return config

python/lsst/pex/config/configChoiceField.py

@@ -46,9 +46,9 @@ class SelectionSet(collections.abc.MutableSet):
     ----------
     dict_ : `ConfigInstanceDict`
         The dictionary of instantiated configs.
-    value
+    value : `~typing.Any`
         The selected key.
-    at : `lsst.pex.config.callStack.StackFrame`, optional
+    at : `list` of `~lsst.pex.config.callStack.StackFrame` or `None`, optional
         The call stack when the selection was made.
     label : `str`, optional
         Label for history tracking.
@@ -94,7 +94,16 @@ def _config(self) -> Config:
         return self._config_()
 
     def add(self, value, at=None):
-        """Add a value to the selected set."""
+        """Add a value to the selected set.
+
+        Parameters
+        ----------
+        value : `~typing.Any`
+            The selected key.
+        at : `list` of `~lsst.pex.config.callStack.StackFrame` or `None`,\
+                optional
+            Stack frames for history recording.
+        """
         if self._config._frozen:
             raise FieldValidationError(self._field, self._config, "Cannot modify a frozen Config")
 
@@ -109,7 +118,16 @@ def add(self, value, at=None):
         self._set.add(value)
 
     def discard(self, value, at=None):
-        """Discard a value from the selected set."""
+        """Discard a value from the selected set.
+
+        Parameters
+        ----------
+        value : `~typing.Any`
+            The selected key.
+        at : `list` of `~lsst.pex.config.callStack.StackFrame` or `None`,\
+                optional
+            Stack frames for history recording.
+        """
         if self._config._frozen:
             raise FieldValidationError(self._field, self._config, "Cannot modify a frozen Config")
 
@@ -295,7 +313,7 @@ def __setitem__(self, k, value, at=None, label="assignment"):
         except Exception:
             raise FieldValidationError(self._field, self._config, "Unknown key %r" % k)
 
-        if value != dtype and type(value) != dtype:
+        if value != dtype and type(value) is not dtype:
             msg = "Value {} at key {} is of incorrect type {}. Expected type {}".format(
                 value,
                 k,

python/lsst/pex/config/configDictField.py

@@ -38,6 +38,19 @@ class ConfigDict(Dict[str, Config]):
 
     Much like `Dict`, `ConfigDict` is a custom `MutableMapper` which tracks
     the history of changes to any of its items.
+
+    Parameters
+    ----------
+    config : `~lsst.pex.config.Config`
+        Config to use.
+    field : `~lsst.pex.config.ConfigDictField`
+        Field to use.
+    value : `~typing.Any`
+        Value to store in dict.
+    at : `list` of `~lsst.pex.config.callStack.StackFrame` or `None`, optional
+        Stack frame for history recording. Will be calculated if `None`.
+    label : `str`, optional
+        Label to use for history recording.
     """
 
     def __init__(self, config, field, value, at, label):
@@ -51,15 +64,15 @@ def __setitem__(self, k, x, at=None, label="setitem", setHistory=True):
 
         # validate keytype
         k = _autocast(k, self._field.keytype)
-        if type(k) != self._field.keytype:
+        if type(k) is not self._field.keytype:
             msg = "Key {!r} is of type {}, expected type {}".format(
                 k, _typeStr(k), _typeStr(self._field.keytype)
             )
             raise FieldValidationError(self._field, self._config, msg)
 
         # validate itemtype
         dtype = self._field.itemtype
-        if type(x) != self._field.itemtype and x != self._field.itemtype:
+        if type(x) is not self._field.itemtype and x != self._field.itemtype:
             msg = "Value {} at key {!r} is of incorrect type {}. Expected type {}".format(
                 x,
                 k,
@@ -116,6 +129,10 @@ class ConfigDictField(DictField):
     optional : `bool`, optional
         If `True`, this configuration `~lsst.pex.config.Field` is *optional*.
         Default is `True`.
+    dictCheck : `~collections.abc.Callable` or `None`, optional
+        Callable to check a dict.
+    itemCheck : `~collections.abc.Callable` or `None`, optional
+        Callable to check an item.
     deprecated : None or `str`, optional
         A description of why this Field is deprecated, including removal date.
         If not None, the string is appended to the docstring for this Field.

python/lsst/pex/config/configField.py

@@ -126,7 +126,7 @@ def __set__(
             raise FieldValidationError(self, instance, "Cannot modify a frozen Config")
         name = _joinNamePath(prefix=instance._name, name=self.name)
 
-        if value != self.dtype and type(value) != self.dtype:
+        if value != self.dtype and type(value) is not self.dtype:
             msg = "Value {} is of incorrect type {}. Expected {}".format(
                 value,
                 _typeStr(value),