From 07f29513c7177c27a59c3fe1762a895d91409d59 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 12 Jun 2023 17:33:23 -0400 Subject: [PATCH] docs: Fill out some API docs (#98) Signed-off-by: Henry Schreiner --- .github/workflows/ci.yml | 23 +++++++++++++ docs/api/repo_review.rst | 58 +++++++++++++++++++++++++++++++ docs/checks.md | 4 +-- docs/conf.py | 17 ++++++++- docs/index.md | 8 +++++ docs/intro.md | 2 +- docs/plugins.md | 4 +-- noxfile.py | 44 +++++++++++++++++++++-- pyproject.toml | 2 ++ src/repo_review/__init__.py | 2 +- src/repo_review/__main__.py | 6 ++-- src/repo_review/checks.py | 58 +++++++++++++++++++++++++------ src/repo_review/families.py | 19 ++++++++-- src/repo_review/fixtures.py | 53 +++++++++++++++++++++------- src/repo_review/ghpath.py | 40 +++++++++++++++++++++ src/repo_review/html.py | 5 ++- src/repo_review/processor.py | 67 ++++++++++++++++++++++++------------ 17 files changed, 354 insertions(+), 58 deletions(-) create mode 100644 docs/api/repo_review.rst diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44e16f2..d70369b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -72,6 +72,29 @@ jobs: - uses: hynek/build-and-inspect-python-package@v1 + docs: + name: Docs + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + + - uses: wntrblm/nox@2023.04.22 + with: + python-versions: "3.11" + + - name: Linkcheck + run: nox -s docs -- -b linkcheck + + - name: Build docs with warnings as errors + run: nox -s docs -- -W + + - name: Verify no changes required to API docs + run: | + nox -s build_api_docs + git diff --exit-code + action: name: Action runs-on: ubuntu-latest diff --git a/docs/api/repo_review.rst b/docs/api/repo_review.rst new file mode 100644 index 0000000..871e03e --- /dev/null +++ b/docs/api/repo_review.rst @@ -0,0 +1,58 @@ +repo\_review package +==================== + +.. automodule:: repo_review + :members: + :undoc-members: + :show-inheritance: + +Submodules +---------- + +repo\_review.checks module +-------------------------- + +.. automodule:: repo_review.checks + :members: + :undoc-members: + :show-inheritance: + +repo\_review.families module +---------------------------- + +.. automodule:: repo_review.families + :members: + :undoc-members: + :show-inheritance: + +repo\_review.fixtures module +---------------------------- + +.. automodule:: repo_review.fixtures + :members: + :undoc-members: + :show-inheritance: + +repo\_review.ghpath module +-------------------------- + +.. automodule:: repo_review.ghpath + :members: + :undoc-members: + :show-inheritance: + +repo\_review.html module +------------------------ + +.. automodule:: repo_review.html + :members: + :undoc-members: + :show-inheritance: + +repo\_review.processor module +----------------------------- + +.. automodule:: repo_review.processor + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/checks.md b/docs/checks.md index 2ba3d7a..ea298ff 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -24,7 +24,7 @@ class Check: ``` You need to implement `family`, which is a string indicating which family it is -grouped under, and `check()`, which can take [](fixtures), and returns `True` if +grouped under, and `check()`, which can take [](./fixtures.md), and returns `True` if the check passes, or `False` if the check fails. If you want a dynamic error explanation instead of the `check()` docstring, you can return a non-empty string from the check instead of `False`. Returning `None` makes a check @@ -89,7 +89,7 @@ Key features: You register checks with a function that returns a dict of checks, with the code of the check (letters + number) as the key, and check instances as the values. -This function can take [](fixtures), as well, allowing customization of checks +This function can take [](./fixtures.md), as well, allowing customization of checks based on repo properties. Here is the suggested function for the above example: diff --git a/docs/conf.py b/docs/conf.py index 2f61b31..684a9f9 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -8,9 +8,12 @@ extensions = [ "myst_parser", + "sphinx.ext.autodoc", + "sphinx.ext.intersphinx", + "sphinx_autodoc_typehints", + "sphinx_copybutton", "sphinxcontrib.programoutput", "sphinxext.opengraph", - "sphinx_copybutton", ] source_suffix = [".rst", ".md"] @@ -30,3 +33,15 @@ "colon_fence", "deflist", ] + +intersphinx_mapping = { + "python": ("https://docs.python.org/3", None), +} + +nitpick_ignore = [ + ("py:class", "_io.StringIO"), + ("py:class", "_io.BytesIO"), + ("py:class", "repo_review.fixtures.T"), +] + +always_document_param_types = True diff --git a/docs/index.md b/docs/index.md index 9427c29..0a6661c 100644 --- a/docs/index.md +++ b/docs/index.md @@ -13,6 +13,14 @@ families webapp ``` +```{toctree} +:maxdepth: 2 +:hidden: +:caption: API + +api/repo_review +``` + ```{include} ../README.md :start-after: ``` diff --git a/docs/intro.md b/docs/intro.md index 6dda321..c19be7d 100644 --- a/docs/intro.md +++ b/docs/intro.md @@ -27,7 +27,7 @@ Plugins are also encouraged to support pre-commit and GitHub Actions. ## Running checks You can run checks with (`pipx run`) `repo-review ` or `python -m -repo_review `. See [](cli) for command-line options. +repo_review `. See [](./cli.md) for command-line options. ## Configuring diff --git a/docs/plugins.md b/docs/plugins.md index b6900e1..40718f8 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -1,7 +1,7 @@ # Writing a plugin -To write a plugin for repo-review, you should provide one or more [](checks). -You can also add new [](fixtures), and customize [](families) with sorting and +To write a plugin for repo-review, you should provide one or more [](./checks.md). +You can also add new [](./fixtures.md), and customize [](./families.md) with sorting and nicer display names. When writing a plugin, you should also do a few things when setting up the package. These suggestions assume you are using a standardized backend, such as `hatchling`, `flit-core`, `pdm-backend`, or diff --git a/noxfile.py b/noxfile.py index 0cad1d1..4dbfafb 100644 --- a/noxfile.py +++ b/noxfile.py @@ -84,12 +84,52 @@ def docs(session: nox.Session) -> None: parser = argparse.ArgumentParser() parser.add_argument("--serve", action="store_true", help="Serve after building") - args = parser.parse_args(session.posargs) + parser.add_argument( + "-b", dest="builder", default="html", help="Build target (default: html)" + ) + args, posargs = parser.parse_known_args(session.posargs) + + if args.builder != "html" and args.serve: + session.error("Must not specify non-HTML builder with --serve") session.install(".[docs]") session.chdir("docs") - session.run("sphinx-build", "-M", "html", ".", "_build") + + if args.builder == "linkcheck": + session.run( + "sphinx-build", "-b", "linkcheck", ".", "_build/linkcheck", *posargs + ) + return + + session.run( + "sphinx-build", + "-n", # nitpicky mode + "-T", # full tracebacks + "-b", + args.builder, + ".", + f"_build/{args.builder}", + *posargs, + ) if args.serve: session.log("Launching docs at http://localhost:8000/ - use Ctrl-C to quit") session.run("python", "-m", "http.server", "8000", "-d", "_build/html") + + +@nox.session +def build_api_docs(session: nox.Session) -> None: + """ + Build (regenerate) API docs. + """ + session.install("sphinx") + session.chdir("docs") + session.run( + "sphinx-apidoc", + "-o", + "api/", + "--module-first", + "--no-toc", + "--force", + "../src/repo_review", + ) diff --git a/pyproject.toml b/pyproject.toml index 8aed8b0..c248a8e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,7 @@ docs = [ "myst_parser >=0.13", "repo-review[cli]", "sphinx >=4.0", + "sphinx-autodoc-typehints", "sphinx-copybutton", "sphinxcontrib-programoutput", "sphinxext-opengraph", @@ -125,6 +126,7 @@ messages_control.disable = [ "redefined-outer-name", "no-member", # better handled by mypy, etc. "used-before-assignment", # False positive on conditional import + "unnecessary-ellipsis", # Not correct for typing ] diff --git a/src/repo_review/__init__.py b/src/repo_review/__init__.py index 0d343fd..da179cc 100644 --- a/src/repo_review/__init__.py +++ b/src/repo_review/__init__.py @@ -1,7 +1,7 @@ """ Copyright (c) 2022 Henry Schreiner. All rights reserved. -repo-review: Review repos with a set of checks defined by plugins. +Review repos with a set of checks defined by plugins. """ diff --git a/src/repo_review/__main__.py b/src/repo_review/__main__.py index 1ee311a..3ae3a33 100644 --- a/src/repo_review/__main__.py +++ b/src/repo_review/__main__.py @@ -8,8 +8,10 @@ from pathlib import Path from typing import Literal +import click as orig_click + if typing.TYPE_CHECKING: - import click + import click # pylint: disable=reimported else: import rich_click as click @@ -36,7 +38,7 @@ def __dir__() -> list[str]: return __all__ -rich.traceback.install(suppress=[click, rich], show_locals=True, width=None) +rich.traceback.install(suppress=[click, rich, orig_click], show_locals=True, width=None) def list_all(ctx: click.Context, _param: click.Parameter, value: bool) -> None: diff --git a/src/repo_review/checks.py b/src/repo_review/checks.py index 3108e70..4630f4c 100644 --- a/src/repo_review/checks.py +++ b/src/repo_review/checks.py @@ -10,15 +10,51 @@ class Check(Protocol): - family: str - requires: Set[str] = frozenset() # Optional - url: str = "" # Optional + """ + This is the check Protocol. Since Python doesn't support optional Protocol + members, the two optional members are required if you want to use this + Protocol in a type checker. The members can be specified as class + properties if you want. + """ + + @property + def family(self) -> str: + """ + The family is a string that the checks will be grouped by. + """ + + @property + def requires(self) -> Set[str]: # Optional + """ + Requires is an (optional) set of checks that must pass for this check + to run. Omitting this is like returning `set()`. + """ + + @property + def url(self) -> str: # Optional + """ + This is an (optional) URL to link to for this check. An empty string is + identical to omitting this member. + """ def check(self) -> bool | None | str: + """ + This is a check. The docstring is used as the failure message if + `False` is returned. Returning None is a skip. Returning `True` (or an + empty string) is a pass. Can be a :func:`classmethod` or + :func:`staticmethod`. Can take fixtures. + """ ... def collect_checks(fixtures: Mapping[str, Any]) -> dict[str, Check]: + """ + Produces a list of checks based on installed entry points. You must provide + the evaluated fixtures so that the check functions have access to the + fixtures when they are running. + + :param fixtures: Fully evaluated dict of fixtures. + """ check_functions = ( ep.load() for ep in importlib.metadata.entry_points(group="repo_review.checks") ) @@ -30,18 +66,20 @@ def collect_checks(fixtures: Mapping[str, Any]) -> dict[str, Check]: } -def is_allowed(select_list: Set[str], ignore_list: Set[str], name: str) -> bool: +def is_allowed(select: Set[str], ignore: Set[str], name: str) -> bool: """ Skips the check if the name is in the ignore list or if the name without the number is in the ignore list. If the select list is not empty, only runs the check if the name or name without the number is in the select list. + + :param select: A set of names or prefixes to include. + :param ignore: A set of names or prefixes to exclude. + :param name: The check to test. + + :return: True if this check is allowed, False otherwise. """ - if ( - select_list - and name not in select_list - and name.rstrip("0123456789") not in select_list - ): + if select and name not in select and name.rstrip("0123456789") not in select: return False - if name in ignore_list or name.rstrip("0123456789") in ignore_list: + if name in ignore or name.rstrip("0123456789") in ignore: return False return True diff --git a/src/repo_review/families.py b/src/repo_review/families.py index 4f633be..13d6541 100644 --- a/src/repo_review/families.py +++ b/src/repo_review/families.py @@ -11,11 +11,26 @@ def __dir__() -> list[str]: class Family(typing.TypedDict, total=False): - name: str # defaults to key - order: int # defaults to 0 + """ + A typed Dict that is used to customize the display of families in reports. + """ + + #: Optional nice name to display instead of family key. Treated like family + #: key if missing. + name: str + + #: Checks are first sorted by this integer order, then alphabetically by + #: family key. Treated like 0 if missing. + order: int def collect_families() -> dict[str, Family]: + """ + Produces a dict mapping family keys to :class:`Family` dicts based on installed + entry points. Unlike other similar functions, this one currently does not + expect a dict of fixtures; dynamic listing is not usually needed for + :class:`Family`'s. + """ return { name: family for ep in importlib.metadata.entry_points(group="repo_review.families") diff --git a/src/repo_review/fixtures.py b/src/repo_review/fixtures.py index 72b3d80..dc8ca94 100644 --- a/src/repo_review/fixtures.py +++ b/src/repo_review/fixtures.py @@ -24,7 +24,10 @@ def __dir__() -> list[str]: def pyproject(package: Traversable) -> dict[str, Any]: """ - The pyproject.toml structure from the package. + Fixture: The ``pyproject.toml`` structure from the package. Returned an + empty dict if no pyproject.toml found. + + :param package: The package fixture. """ pyproject_path = package.joinpath("pyproject.toml") if pyproject_path.is_file(): @@ -34,38 +37,64 @@ def pyproject(package: Traversable) -> dict[str, Any]: def compute_fixtures( - root: Traversable, package: Traversable, fixtures: Mapping[str, Callable[..., Any]] + root: Traversable, + package: Traversable, + unevaluated_fixtures: Mapping[str, Callable[..., Any]], ) -> dict[str, Any]: - results: dict[str, Any] = {"root": root, "package": package} + """ + Given the repo ``root`` Traversable, the ``package`` Traversable, and the dict + of all fixture callables, compute the dict of fixture results. + + :param root: The root of the repository + :param package: The path to the package (``root / subdir``) + :param unevaluated_fixtures: The unevaluated mapping of fixture names to + callables. + + :return: The fully evaluated dict of fixtures. + """ + fixtures: dict[str, Any] = {"root": root, "package": package} graph: dict[str, Set[str]] = {"root": set(), "package": set()} graph |= { - name: inspect.signature(fix).parameters.keys() for name, fix in fixtures.items() + name: inspect.signature(fix).parameters.keys() + for name, fix in unevaluated_fixtures.items() } ts = graphlib.TopologicalSorter(graph) for fixture_name in ts.static_order(): if fixture_name in {"package", "root"}: continue - func = fixtures[fixture_name] + func = unevaluated_fixtures[fixture_name] signature = inspect.signature(func) - kwargs = {name: results[name] for name in signature.parameters} - results[fixture_name] = fixtures[fixture_name](**kwargs) - return results + kwargs = {name: fixtures[name] for name in signature.parameters} + fixtures[fixture_name] = unevaluated_fixtures[fixture_name](**kwargs) + return fixtures T = typing.TypeVar("T") -def apply_fixtures(computed_fixtures: Mapping[str, Any], func: Callable[..., T]) -> T: +def apply_fixtures(fixtures: Mapping[str, Any], func: Callable[..., T]) -> T: + """ + Given the pre-computed dict of fixtures and a function, fill in any + fixtures from that dict that it requests and return the result. + + :param fixtures: Fully evaluated dict of fixtures. + :param func: Some callable that can take fixtures. + """ signature = inspect.signature(func) kwargs = { - name: value - for name, value in computed_fixtures.items() - if name in signature.parameters + name: value for name, value in fixtures.items() if name in signature.parameters } return func(**kwargs) def collect_fixtures() -> dict[str, Callable[[Traversable], Any]]: + """ + Produces a dict of fixture callables based on installed entry points. You + should call :func:`compute_fixtures` on the result to get the standard dict of + fixture results that most other functions in repo-review expect. + + :return: A dict of unevaluated fixtures. + """ return { ep.name: ep.load() for ep in importlib.metadata.entry_points(group="repo_review.fixtures") diff --git a/src/repo_review/ghpath.py b/src/repo_review/ghpath.py index fec8b0b..583a9a4 100644 --- a/src/repo_review/ghpath.py +++ b/src/repo_review/ghpath.py @@ -22,9 +22,25 @@ def __dir__() -> list[str]: @dataclasses.dataclass(frozen=True, kw_only=True) class GHPath(Traversable): + """ + This is a Traversable that can be used to navigate a GitHub repo without + downloading it. + + :param repo: The repo name, in "org/repo" style. + :param branch: The branch name. Required, even if using the default branch. + :param path: A sub-path inside the repo. Defaults to the repo root. + :param _info: Some internal info stored to keep accesses fast. + """ + + #: The repository name, in `"org/repo"` style. repo: str + + #: The branch name. Required, even if using the default branch. branch: str + + #: A path inside the repo path: str = "" + _info: list[dict[str, str]] = dataclasses.field( hash=False, default_factory=list, repr=False ) @@ -54,6 +70,9 @@ def __str__(self) -> str: @property def name(self) -> str: + """ + The final element of the path or the repo name. + """ return (self.path or self.repo).split("/")[-1] @typing.overload # type: ignore[override] @@ -67,6 +86,13 @@ def open(self, mode: Literal["rb"]) -> io.BytesIO: def open( self, mode: Literal["r", "rb"] = "r", encoding: str | None = "utf-8" ) -> io.IOBase: + """ + Open the repo. This doesn't support the full collection of options, + only utf-8 and binary. + + :param mode: The mode, only ``"r"`` or ``"rb"`` supported. + :param encoding: The encoding, only ``"utf-8"`` or ``None`` supported. + """ assert encoding is None or encoding == "utf-8", "Only utf-8 is supported" val: io.StringIO = self.open_url( f"https://raw.githubusercontent.com/{self.repo}/{self.branch}/{self.path}" @@ -113,7 +139,18 @@ def read_bytes(self) -> bytes: @dataclasses.dataclass(frozen=True, kw_only=True) class EmptyTraversable(Traversable): + """ + This is a Traversable representing an empty directory or a non-existent + file. + + :param is_a_dir: True to treat this like an empty dir. + :param _fake_name: A customisable fake name. + """ + + #: True if this is supposed to be a directory is_a_dir: bool = True + + #: Customizable fake name _fake_name: str = "not-a-real-path" def __str__(self) -> str: @@ -121,6 +158,9 @@ def __str__(self) -> str: @property def name(self) -> str: + """ + Return a dummy name. + """ return self._fake_name @typing.overload # type: ignore[override] diff --git a/src/repo_review/html.py b/src/repo_review/html.py index ff6fa1d..14bfdc8 100644 --- a/src/repo_review/html.py +++ b/src/repo_review/html.py @@ -20,7 +20,10 @@ def __dir__() -> list[str]: def to_html(families: Mapping[str, Family], processed: list[Result]) -> str: """ - Convert the results of a repo review to HTML. + Convert the results of a repo review (``families``, ``processed``) to HTML. + + :param families: The family mapping. + :param processed: The list of processed results. """ out = io.StringIO() print = functools.partial(builtins.print, file=out) diff --git a/src/repo_review/processor.py b/src/repo_review/processor.py index 92b592c..4ea06b0 100644 --- a/src/repo_review/processor.py +++ b/src/repo_review/processor.py @@ -31,35 +31,54 @@ def __dir__() -> list[str]: md = markdown_it.MarkdownIt() -# Helper to get the type in the JSON style returns class ResultDict(typing.TypedDict): - family: str - description: str - result: bool | None - err_msg: str - url: str + """ + Helper to get the type in the JSON style returns. Basically identical to + :class:`Result` but in dict form and without the name. + """ + + family: str #: The family string + description: str #: The short description of what the check looks for + result: bool | None #: The result, None means skip + err_msg: str #: The error message if the result is false, in markdown format + url: str #: An optional URL (empty string if missing) @dataclasses.dataclass(frozen=True, kw_only=True) class Result: - family: str - name: str - description: str - result: bool | None - err_msg: str = "" - url: str = "" + """ + This is the returned value from a processed check. + """ + + family: str #: The family string + name: str #: The name of the check + description: str #: The short description of what the check looks for + result: bool | None #: The result, None means skip + err_msg: str = "" #: The error message if the result is false, in markdown format + url: str = "" #: An optional URL (empty string if missing) def err_markdown(self) -> str: + """ + Produces HTML from the error message, assuming it is in markdown. + """ result: str = md.render(self.err_msg).strip() return result.removeprefix("

").removesuffix("

").strip() class ProcessReturn(typing.NamedTuple): - families: dict[str, Family] - results: list[Result] + """ + Return type for :func:`process`. + """ + + families: dict[str, Family] #: A mapping of family strings to Family info dicts + results: list[Result] #: The results list class HasFamily(typing.Protocol): + """ + Simple Protocol to see if family property is present. + """ + @property def family(self) -> str: ... @@ -115,16 +134,14 @@ def process( """ Process the package and return a dictionary of results. - Parameters - ---------- - root: Traversable | Path - The Path(like) to the repository to process + :param root: The Path(like) to the repository to process - ignore: Sequence[str] - A list of checks to ignore + :param ignore: A list of checks to ignore - subidr: str - The path to the package in the subdirectory, if not at the root of the repository. + :param subdir: The path to the package in the subdirectory, if not at the + root of the repository. + + :return: A dictionary of check results. """ package = root.joinpath(subdir) if subdir else root @@ -179,6 +196,12 @@ def process( def as_simple_dict(results: list[Result]) -> dict[str, ResultDict]: + """ + Convert a results list into a simple dict of dicts structure. The name of + the result turns into the key of the outer dict. + + :param results: The list of results. + """ return { result.name: typing.cast( ResultDict,