Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify CLIs #537

Merged
merged 11 commits into from
Apr 30, 2024
16 changes: 8 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ jobs:

- name: Make sure CLI works
run: |
python -m numpydoc numpydoc.tests.test_main._capture_stdout
echo '! python -m numpydoc numpydoc.tests.test_main._invalid_docstring' | bash
python -m numpydoc --validate numpydoc.tests.test_main._capture_stdout
echo '! python -m numpydoc --validate numpydoc.tests.test_main._docstring_with_errors' | bash
numpydoc render numpydoc.tests.test_main._capture_stdout
echo '! numpydoc render numpydoc.tests.test_main._invalid_docstring' | bash
numpydoc validate numpydoc.tests.test_main._capture_stdout
echo '! numpydoc validate numpydoc.tests.test_main._docstring_with_errors' | bash

- name: Setup for doc build
run: |
Expand Down Expand Up @@ -101,10 +101,10 @@ jobs:

- name: Make sure CLI works
run: |
python -m numpydoc numpydoc.tests.test_main._capture_stdout
echo '! python -m numpydoc numpydoc.tests.test_main._invalid_docstring' | bash
python -m numpydoc --validate numpydoc.tests.test_main._capture_stdout
echo '! python -m numpydoc --validate numpydoc.tests.test_main._docstring_with_errors' | bash
numpydoc render numpydoc.tests.test_main._capture_stdout
echo '! numpydoc render numpydoc.tests.test_main._invalid_docstring' | bash
numpydoc validate numpydoc.tests.test_main._capture_stdout
echo '! numpydoc validate numpydoc.tests.test_main._docstring_with_errors' | bash

- name: Setup for doc build
run: |
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
- id: numpydoc-validation
name: numpydoc-validation
description: This hook validates that docstrings in committed files adhere to numpydoc standards.
entry: validate-docstrings
entry: numpydoc lint
require_serial: true
language: python
types: [python]
6 changes: 3 additions & 3 deletions doc/validation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ command line options for this hook:

.. code-block:: bash

$ python -m numpydoc.hooks.validate_docstrings --help
$ numpydoc lint --help

Using a config file provides additional customization. Both ``pyproject.toml``
and ``setup.cfg`` are supported; however, if the project contains both
Expand Down Expand Up @@ -102,12 +102,12 @@ can be called. For example, to do it for ``numpy.ndarray``, use:

.. code-block:: bash

$ python -m numpydoc numpy.ndarray
$ numpydoc validate numpy.ndarray

This will validate that the docstring can be built.

For an exhaustive validation of the formatting of the docstring, use the
``--validate`` parameter. This will report the errors detected, such as
``validate`` subcommand. This will report the errors detected, such as
incorrect capitalization, wrong order of the sections, and many other
issues. Note that this will honor :ref:`inline ignore comments <inline_ignore_comments>`,
but will not look for any configuration like the :ref:`pre-commit hook <pre_commit_hook>`
Expand Down
53 changes: 2 additions & 51 deletions numpydoc/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,6 @@
Implementing `python -m numpydoc` functionality.
"""

import sys
import argparse
import ast
from .cli import main

from .docscrape_sphinx import get_doc_object
from .validate import validate, Validator


def render_object(import_path, config=None):
"""Test numpydoc docstring generation for a given object"""
# TODO: Move Validator._load_obj to a better place than validate
print(get_doc_object(Validator._load_obj(import_path), config=dict(config or [])))
return 0


def validate_object(import_path):
exit_status = 0
results = validate(import_path)
for err_code, err_desc in results["errors"]:
exit_status += 1
print(":".join([import_path, err_code, err_desc]))
return exit_status


if __name__ == "__main__":
ap = argparse.ArgumentParser(description=__doc__)
ap.add_argument("import_path", help="e.g. numpy.ndarray")

def _parse_config(s):
key, _, value = s.partition("=")
value = ast.literal_eval(value)
return key, value

ap.add_argument(
"-c",
"--config",
type=_parse_config,
action="append",
help="key=val where val will be parsed by literal_eval, "
"e.g. -c use_plots=True. Multiple -c can be used.",
)
ap.add_argument(
"--validate", action="store_true", help="validate the object and report errors"
)
args = ap.parse_args()

if args.validate:
exit_code = validate_object(args.import_path)
else:
exit_code = render_object(args.import_path, args.config)

sys.exit(exit_code)
raise SystemExit(main())
stefmolin marked this conversation as resolved.
Show resolved Hide resolved
68 changes: 68 additions & 0 deletions numpydoc/cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
"""The CLI for numpydoc."""

import argparse
import ast
from typing import List, Sequence, Union

from .docscrape_sphinx import get_doc_object
from .hooks import validate_docstrings
from .validate import Validator, validate


def render_object(import_path: str, config: Union[List[str], None] = None) -> int:
"""Test numpydoc docstring generation for a given object."""
# TODO: Move Validator._load_obj to a better place than validate
print(get_doc_object(Validator._load_obj(import_path), config=dict(config or [])))
return 0


def validate_object(import_path: str) -> int:
"""Run numpydoc docstring validation for a given object."""
exit_status = 0
results = validate(import_path)
for err_code, err_desc in results["errors"]:
exit_status += 1
print(":".join([import_path, err_code, err_desc]))
return exit_status


def main(argv: Union[Sequence[str], None] = None) -> int:
"""CLI for numpydoc."""
ap = argparse.ArgumentParser(prog="numpydoc", description=__doc__)
subparsers = ap.add_subparsers(title="subcommands")

def _parse_config(s):
key, _, value = s.partition("=")
value = ast.literal_eval(value)
return key, value

render = subparsers.add_parser(
"render",
description="Test numpydoc docstring generation for a given object.",
stefmolin marked this conversation as resolved.
Show resolved Hide resolved
help="generate the docstring with numpydoc",
)
render.add_argument("import_path", help="e.g. numpy.ndarray")
render.add_argument(
"-c",
"--config",
type=_parse_config,
action="append",
help="key=val where val will be parsed by literal_eval, "
"e.g. -c use_plots=True. Multiple -c can be used.",
)
render.set_defaults(func=render_object)

validate = subparsers.add_parser(
"validate",
description="Validate the docstring with numpydoc.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clarify here the distinction between validate and lint. They do the same, except the one operates on a function and the other on a file? But validate doesn't allow disabling certain rules, which confused me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it works for non-functions as well. How about this?

Validate an object's docstring conforms to the numpydoc standard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what to do about the second part of your comment. If you run numpydoc validate --help you see what can be passed as arguments, and there aren't any for disabling rules.

Copy link
Contributor

@stefanv stefanv Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Validate that an object's docstring conforms to the numpydoc standard" (added "that")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, "Determine (or check?) whether an object's docstring..."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise just: "Validate an object's docstring against the numpydoc standard"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may prefer the last version, but anything gramatically sound is fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what to do about the second part of your comment. If you run numpydoc validate --help you see what can be passed as arguments, and there aren't any for disabling rules.

This was more of a question: should there be the option of disabling certain rules? Why are these not exactly equivalent in their functionality?

Copy link
Contributor Author

@stefmolin stefmolin Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what to do about the second part of your comment. If you run numpydoc validate --help you see what can be passed as arguments, and there aren't any for disabling rules.

This was more of a question: should there be the option of disabling certain rules? Why are these not exactly equivalent in their functionality?

While I agree this should eventually be the case, this definitely seems out of scope for this PR. At least the separate subcommands will not draw attention to arguments that have no effect like before (see #459).

help="validate the object and report errors",
)
validate.add_argument("import_path", help="e.g. numpy.ndarray")
validate.set_defaults(func=validate_object)

lint_parser = validate_docstrings.get_parser(parent=subparsers)
stefmolin marked this conversation as resolved.
Show resolved Hide resolved
lint_parser.set_defaults(func=validate_docstrings.run_hook)

args = vars(ap.parse_args(argv))
func = args.pop("func", render_object)
return func(**args)
74 changes: 59 additions & 15 deletions numpydoc/hooks/validate_docstrings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import tomli as tomllib

from pathlib import Path
from typing import Sequence, Tuple, Union
from typing import Any, Dict, List, Tuple, Union

from tabulate import tabulate

Expand Down Expand Up @@ -341,9 +341,22 @@ def process_file(filepath: os.PathLike, config: dict) -> "list[list[str]]":
return docstring_visitor.findings


def main(argv: Union[Sequence[str], None] = None) -> int:
"""Run the numpydoc validation hook."""
def get_parser(
parent: Union[argparse.ArgumentParser, None] = None,
) -> argparse.ArgumentParser:
"""
Build an argument parser.

Parameters
----------
parent : argparse.ArgumentParser
A parent parser to use, if this should be a subparser.

Returns
-------
argparse.ArgumentParser
The argument parser.
"""
project_root_from_cwd, config_file = find_project_root(["."])
config_options = parse_config(project_root_from_cwd)
ignored_checks = (
Expand All @@ -357,10 +370,21 @@ def main(argv: Union[Sequence[str], None] = None) -> int:
+ "\n"
)

parser = argparse.ArgumentParser(
description="Run numpydoc validation on files with option to ignore individual checks.",
formatter_class=argparse.RawTextHelpFormatter,
description = (
"Run numpydoc validation on files with option to ignore individual checks."
)
if parent is None:
stefmolin marked this conversation as resolved.
Show resolved Hide resolved
parser = argparse.ArgumentParser(
description=description,
formatter_class=argparse.RawTextHelpFormatter,
)
else:
parser = parent.add_parser(
"lint",
description=description,
help="validate all docstrings in file(s) using the abstract syntax tree",
)

parser.add_argument(
"files", type=str, nargs="+", help="File(s) to run numpydoc validation on."
)
Expand Down Expand Up @@ -389,14 +413,38 @@ def main(argv: Union[Sequence[str], None] = None) -> int:
}"""
),
)
return parser


args = parser.parse_args(argv)
project_root, _ = find_project_root(args.files)
config_options = parse_config(args.config or project_root)
config_options["checks"] -= set(args.ignore or [])
def run_hook(
files: List[str],
*,
config: Union[Dict[str, Any], None] = None,
ignore: Union[List[str], None] = None,
) -> int:
"""
Run the numpydoc validation hook.

Parameters
----------
files : list[str]
The absolute or relative paths to the files to inspect.
config : Union[dict[str, Any], None], optional
Configuration options for reviewing flagged issues.
ignore : Union[list[str], None], optional
Checks to ignore in the results.

Returns
-------
int
The return status: 1 if issues were found, 0 otherwise.
"""
project_root, _ = find_project_root(files)
config_options = parse_config(config or project_root)
config_options["checks"] -= set(ignore or [])

findings = []
for file in args.files:
for file in files:
findings.extend(process_file(file, config_options))

if findings:
Expand All @@ -411,7 +459,3 @@ def main(argv: Union[Sequence[str], None] = None) -> int:
)
return 1
return 0


if __name__ == "__main__":
raise SystemExit(main())
22 changes: 14 additions & 8 deletions numpydoc/tests/hooks/test_validate_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import pytest

from numpydoc.hooks.validate_docstrings import main
from numpydoc.hooks.validate_docstrings import get_parser, run_hook


@pytest.fixture
Expand All @@ -20,6 +20,11 @@ def example_module(request):
return str(fullpath.relative_to(request.config.rootdir))


def hook_main(args):
parser = get_parser()
return run_hook(**vars(parser.parse_args(args)))


@pytest.mark.parametrize("config", [None, "fake_dir"])
def test_validate_hook(example_module, config, capsys):
"""Test that a file is correctly processed in the absence of config files."""
Expand Down Expand Up @@ -76,7 +81,7 @@ def test_validate_hook(example_module, config, capsys):
if config:
args.append(f"--{config=}")

return_code = main(args)
return_code = hook_main(args)
assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected

Expand Down Expand Up @@ -109,7 +114,8 @@ def test_validate_hook_with_ignore(example_module, capsys):
"""
)

return_code = main([example_module, "--ignore", "ES01", "SA01", "EX01"])
return_code = hook_main([example_module, "--ignore", "ES01", "SA01", "EX01"])

assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected

Expand Down Expand Up @@ -157,7 +163,7 @@ def test_validate_hook_with_toml_config(example_module, tmp_path, capsys):
"""
)

return_code = main([example_module, "--config", str(tmp_path)])
return_code = hook_main([example_module, "--config", str(tmp_path)])
assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected

Expand Down Expand Up @@ -196,7 +202,7 @@ def test_validate_hook_with_setup_cfg(example_module, tmp_path, capsys):
"""
)

return_code = main([example_module, "--config", str(tmp_path)])
return_code = hook_main([example_module, "--config", str(tmp_path)])
assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected

Expand All @@ -205,7 +211,7 @@ def test_validate_hook_help(capsys):
"""Test that help section is displaying."""

with pytest.raises(SystemExit):
return_code = main(["--help"])
return_code = hook_main(["--help"])
assert return_code == 0

out = capsys.readouterr().out
Expand Down Expand Up @@ -255,7 +261,7 @@ def test_validate_hook_exclude_option_pyproject(example_module, tmp_path, capsys
"""
)

return_code = main([example_module, "--config", str(tmp_path)])
return_code = hook_main([example_module, "--config", str(tmp_path)])
assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected

Expand Down Expand Up @@ -292,6 +298,6 @@ def test_validate_hook_exclude_option_setup_cfg(example_module, tmp_path, capsys
"""
)

return_code = main([example_module, "--config", str(tmp_path)])
return_code = hook_main([example_module, "--config", str(tmp_path)])
assert return_code == 1
assert capsys.readouterr().err.rstrip() == expected
Loading
Loading