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

DM-42116: Add numpydoc validation

.github/workflows/docstyle.yaml

@@ -11,3 +11,20 @@ jobs:
     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")
+          python -m numpydoc.hooks.validate_docstrings $(find tests -name "*.py")

.pre-commit-config.yaml

@@ -7,7 +7,7 @@ repos:
       - id: trailing-whitespace
       - id: check-toml
   - repo: https://github.com/psf/black-pre-commit-mirror
-    rev: 23.11.0
+    rev: 23.12.0
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python
@@ -25,3 +25,7 @@ repos:
     rev: v0.1.7
     hooks:
       - id: ruff
+  - repo: https://github.com/numpy/numpydoc
+    rev: "v1.6.0"
+    hooks:
+      - id: numpydoc-validation

pyproject.toml

@@ -171,3 +171,22 @@ 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.
+]
+exclude = [
+    '^__init__$',
+    '^commands\.',  # Click docstrings, not numpydoc
+    '\._[a-zA-Z_]+$',  # Private methods.
+]

python/lsst/ctrl/mpexec/cli/opt/options.py

@@ -467,7 +467,16 @@ def parse_mock_failure(
     ctx: click.Context, param: click.Option, value: Iterable[str] | None
 ) -> Mapping[str, tuple[str, type[Exception] | None]]:
     """Parse the --mock-failure option values into the mapping accepted by
-    `lsst.pipe.base.tests.mocks.mock_task_defs`.
+    `~lsst.pipe.base.tests.mocks.mock_task_defs`.
+
+    Parameters
+    ----------
+    ctx : `click.Context`
+        Context provided by Click.
+    param : `click.Option`
+        Click option.
+    value : `~collections.abc.Iterable` [`str`] or `None`
+        Value from option.
     """
     result: dict[str, tuple[str, type[Exception] | None]] = {}
     if value is None:

python/lsst/ctrl/mpexec/cli/pipetask.py

@@ -48,7 +48,7 @@ class PipetaskCLI(LoaderCLI):
 @log_file_option()
 @log_tty_option()
 @log_label_option()
-def cli(log_level, long_log, log_file, log_tty, log_label) -> None:  # type: ignore
+def cli(log_level, long_log, log_file, log_tty, log_label) -> None:  # type: ignore  # numpydoc ignore=PR01
     """Implement pipetask command line."""
     # log_level is handled by get_command and list_commands, and is called in
     # one of those functions before this is called.

python/lsst/ctrl/mpexec/cli/script/build.py

@@ -58,12 +58,12 @@ def build(  # type: ignore
     order_pipeline : `bool`
         If true, order tasks in pipeline based on their data dependencies,
         ordering is performed as last step before saving or executing pipeline.
+    pipeline : `str`
+        Path location of a pipeline definition file in YAML format.
     pipeline_actions : `list` [`PipelineAction`]] or `PipelineAction`
         A list of pipeline actions in the order they should be executed.
     pipeline_dot : `str`
         Path location for storing GraphViz DOT representation of a pipeline.
-    pipeline : `str`
-        Path location of a pipeline definition file in YAML format.
     save_pipeline : `str`
         Path location for storing resulting pipeline definition in YAML format.
     show : `lsst.ctrl.mpexec.showInfo.ShowInfo`

python/lsst/ctrl/mpexec/cli/script/cleanup.py

@@ -36,7 +36,13 @@
 
 
 class NoSuchCollectionFailure:
-    """Failure when there is no such collection."""
+    """Failure when there is no such collection.
+
+    Parameters
+    ----------
+    collection : `str`
+        Name of collection.
+    """
 
     def __init__(self, collection: str):
         self.collection = collection
@@ -46,7 +52,15 @@ def __str__(self) -> str:
 
 
 class NotChainedCollectionFailure:
-    """Failure when this is not a chained collection."""
+    """Failure when this is not a chained collection.
+
+    Parameters
+    ----------
+    collection : `str`
+        Name of collection.
+    type : `str`
+        Type of collection.
+    """
 
     def __init__(self, collection: str, type: str):
         self.collection = collection
@@ -57,7 +71,13 @@ def __str__(self) -> str:
 
 
 class CleanupResult(ConfirmableResult):
-    """Information containing the result of the cleanup request."""
+    """Information containing the result of the cleanup request.
+
+    Parameters
+    ----------
+    butler_config : `str`
+        Butler configuration URI.
+    """
 
     def __init__(self, butler_config: str):
         self.butler_config = butler_config

python/lsst/ctrl/mpexec/cli/script/confirmable.py

@@ -87,7 +87,20 @@ def can_continue(self) -> bool:
 
 
 def confirm(script_func: Callable[[], ConfirmableResult], confirm: bool) -> ConfirmableResult:
-    """Prompt user to continue."""
+    """Prompt user to continue.
+
+    Parameters
+    ----------
+    script_func : `~collections.abc.Callable`
+        Script function to call.
+    confirm : `bool`
+        Whether confirmation is required.
+
+    Returns
+    -------
+    result : `ConfirmableResult`
+        Confirmation from script function.
+    """
     result = script_func()
     if result.failed:
         raise click.ClickException(result.describe_failure)

python/lsst/ctrl/mpexec/cli/script/purge.py

@@ -41,7 +41,15 @@
 
 
 class ChildHasMultipleParentsFailure:
-    """Failure when the child has multiple parents."""
+    """Failure when the child has multiple parents.
+
+    Parameters
+    ----------
+    child : `str`
+        Child collection name.
+    parents : `list` [`str`]
+        Parent collections.
+    """
 
     def __init__(self, child: str, parents: list[str]):
         self.child = child
@@ -53,7 +61,15 @@ def __str__(self) -> str:
 
 
 class TopCollectionHasParentsFailure:
-    """Failure when the top collection has parents."""
+    """Failure when the top collection has parents.
+
+    Parameters
+    ----------
+    collection : `str`
+        Name of collection.
+    parents : `list` [`str`]
+        Parents of collection.
+    """
 
     def __init__(self, collection: str, parents: list[str]):
         self.collection = collection
@@ -68,7 +84,15 @@ def __str__(self) -> str:
 
 
 class TopCollectionIsNotChainedFailure:
-    """Failure when the top collection is not a chain."""
+    """Failure when the top collection is not a chain.
+
+    Parameters
+    ----------
+    collection : `str`
+        Name of collection.
+    collection_type : `CollectionType`
+        Type of collection.
+    """
 
     def __init__(self, collection: str, collection_type: CollectionType):
         self.collection = collection
@@ -82,7 +106,13 @@ def __str__(self) -> str:
 
 
 class TopCollectionNotFoundFailure:
-    """Failure when the top collection is not found."""
+    """Failure when the top collection is not found.
+
+    Parameters
+    ----------
+    collection : `str`
+        Name of collection.
+    """
 
     def __init__(self, collection: str):
         self.collection = collection
@@ -92,7 +122,13 @@ def __str__(self) -> str:
 
 
 class PurgeResult(ConfirmableResult):
-    """The results of the purge command."""
+    """The results of the purge command.
+
+    Parameters
+    ----------
+    butler_config : `str`
+        Butler configuration URI.
+    """
 
     def __init__(self, butler_config: str):
         self.runs_to_remove: list[str] = []

python/lsst/ctrl/mpexec/cli/script/qgraph.py

@@ -75,7 +75,7 @@ def qgraph(  # type: ignore
 
     Parameters
     ----------
-    pipelineObj : `lsst.pipe.base.Pipeline` or None.
+    pipelineObj : `lsst.pipe.base.Pipeline` or None
         The pipeline object used to generate a qgraph. If this is not `None`
         then `qgraph` should be `None`.
     qgraph : `str` or `None`
@@ -175,7 +175,7 @@ def qgraph(  # type: ignore
         List of overall-input dataset types that should not be mocked.
     mock_failure : `~collections.abc.Sequence`, optional
         List of quanta that should raise exceptions.
-    kwargs : `dict` [`str`, `str`]
+    **kwargs : `dict` [`str`, `str`]
         Ignored; click commands may accept options for more than one script
         function and pass all the option kwargs to each of the script functions
         which ignore these unused kwargs.

python/lsst/ctrl/mpexec/cli/script/report.py

@@ -36,20 +36,20 @@ def report(butler_config: str, qgraph_uri: str, output_yaml: str, logs: bool = T
 
     Parameters
     ----------
-        butler_config : `str`
-            The Butler used for this report. This should match the Butler used
-            for the run associated with the executed quantum graph.
-        qgraph_uri : `str`
-            The uri of the location of said quantum graph.
-        output_yaml : `str`
-            The name to be used for the summary yaml file.
-        logs : `bool`
-            Get butler log datasets for extra information.
+    butler_config : `str`
+        The Butler used for this report. This should match the Butler used
+        for the run associated with the executed quantum graph.
+    qgraph_uri : `str`
+        The uri of the location of said quantum graph.
+    output_yaml : `str`
+        The name to be used for the summary yaml file.
+    logs : `bool`
+        Get butler log datasets for extra information.
 
     See Also
     --------
-        lsst.pipe.base.QuantumGraphExecutionReport.make_reports
-        lsst.pipe.base.QuantumGraphExecutionReport.write_summary_yaml
+    lsst.pipe.base.QuantumGraphExecutionReport.make_reports : Making reports.
+    lsst.pipe.base.QuantumGraphExecutionReport.write_summary_yaml : Summaries.
     """
     butler = Butler.from_config(butler_config, writeable=False)
     qgraph = QuantumGraph.loadUri(qgraph_uri)

python/lsst/ctrl/mpexec/cli/script/run.py

@@ -78,7 +78,7 @@ def run(  # type: ignore
     Parameters
     ----------
     pdb : `bool`
-        Drop into pdb on exception?
+        Drop into pdb on exception or not.
     graph_fixup : `str`
         The name of the class or factory method which makes an instance used
         for execution graph fixup.
@@ -134,7 +134,7 @@ def run(  # type: ignore
         development, but it does not delete the datasets associated with the
         replaced run unless `prune-replaced` is also True. Requires `output`,
         and `extend_run` must be `None`.
-    prune_replaced : "unstore", "purge", or `None`.
+    prune_replaced : "unstore", "purge", or `None`
         If not `None`, delete the datasets in the collection replaced by
         `replace_run`, either just from the datastore ("unstore") or by
         removing them and the RUN completely ("purge"). Requires `replace_run`.
@@ -179,7 +179,7 @@ def run(  # type: ignore
     rebase : `bool`
         If `True` then reset output collection chain if it is inconsistent with
         the ``inputs``.
-    kwargs : `dict` [`str`, `str`]
+    **kwargs : `dict` [`str`, `str`]
         Ignored; click commands may accept options for more than one script
         function and pass all the option kwargs to each of the script functions
         which ignore these unused kwargs.

python/lsst/ctrl/mpexec/cli/script/update_graph_run.py

@@ -47,6 +47,11 @@ def update_graph_run(
         Collection name, if collection exists it must be of ``RUN`` type.
     output_graph : `~lsst.resources.ResourcePathExpression`
         Location to store updated quantum graph.
+    metadata_run_key : `str`
+        Specifies metadata key corresponding to output run name to update
+        with new run name. If metadata is missing it is not
+        updated. If metadata is present but key is missing, it will be
+        added.
     update_graph_id : `bool`
         If `True` then also update graph ID with a new unique value.
     """

python/lsst/ctrl/mpexec/cli/utils.py

@@ -112,19 +112,19 @@ def makePipelineActions(
         passed in on the command line.
     taskFlags : `list` [`str`], optional
         The option flags to use to recognize a task action, by default
-        task_option.opts()
+        task_option.opts().
     deleteFlags : `list` [`str`], optional
         The option flags to use to recognize a delete action, by default
-        delete_option.opts()
+        delete_option.opts().
     configFlags : `list` [`str`], optional
         The option flags to use to recognize a config action, by default
-        config_option.opts()
+        config_option.opts().
     configFileFlags : `list` [`str`], optional
         The option flags to use to recognize a config-file action, by default
-        config_file_option.opts()
+        config_file_option.opts().
     instrumentFlags : `list` [`str`], optional
         The option flags to use to recognize an instrument action, by default
-        instrument_option.opts()
+        instrument_option.opts().
 
     Returns
     -------