From ed7dd0c11e77ff8b74015f5d58b4e583fd64c372 Mon Sep 17 00:00:00 2001 From: Philipp A Date: Tue, 21 Nov 2023 11:33:52 +0100 Subject: [PATCH] Simplify typehints (#105) --- src/scanpydoc/elegant_typehints/__init__.py | 8 +- .../elegant_typehints/_formatting.py | 94 ++--------- .../elegant_typehints/_return_tuple.py | 4 +- .../elegant_typehints/static/typehints.css | 6 - tests/test_elegant_typehints.py | 147 ++++-------------- 5 files changed, 54 insertions(+), 205 deletions(-) delete mode 100644 src/scanpydoc/elegant_typehints/static/typehints.css diff --git a/src/scanpydoc/elegant_typehints/__init__.py b/src/scanpydoc/elegant_typehints/__init__.py index 0c50c9a..953a54b 100644 --- a/src/scanpydoc/elegant_typehints/__init__.py +++ b/src/scanpydoc/elegant_typehints/__init__.py @@ -41,7 +41,7 @@ def x() -> Tuple[int, float]: .. _sphinx issue 4826: https://github.com/sphinx-doc/sphinx/issues/4826 -.. _sphinx-autodoc-typehints issue 38: https://github.com/agronholm/sphinx-autodoc-typehints/issues/38 +.. _sphinx-autodoc-typehints issue 38: https://github.com/tox-dev/sphinx-autodoc-typehints/issues/38 """ # noqa: D300, D301 @@ -97,7 +97,6 @@ def _init_vars(_app: Sphinx, config: Config) -> None: if config.typehints_defaults is None and config.annotate_defaults: # override default for “typehints_defaults” config.typehints_defaults = "braces" - config.html_static_path.append(str(HERE / "static")) @dataclass @@ -112,12 +111,11 @@ def setup(app: Sphinx) -> dict[str, Any]: """Patches :mod:`sphinx_autodoc_typehints` for a more elegant display.""" app.add_config_value("qualname_overrides", default={}, rebuild="html") app.add_config_value("annotate_defaults", default=True, rebuild="html") - app.add_css_file("typehints.css") app.connect("config-inited", _init_vars) - from ._formatting import _role_annot, format_annotation + from ._formatting import _role_annot, typehints_formatter - app.config["typehints_formatter"] = PickleableCallable(format_annotation) + app.config["typehints_formatter"] = PickleableCallable(typehints_formatter) for name in ["annotation-terse", "annotation-full"]: roles.register_canonical_role( name, diff --git a/src/scanpydoc/elegant_typehints/_formatting.py b/src/scanpydoc/elegant_typehints/_formatting.py index 0fb0432..a3f54e7 100644 --- a/src/scanpydoc/elegant_typehints/_formatting.py +++ b/src/scanpydoc/elegant_typehints/_formatting.py @@ -1,12 +1,7 @@ from __future__ import annotations import inspect -from collections.abc import Callable, Iterable, Mapping, Sequence -from functools import partial -from typing import TYPE_CHECKING, Any, Literal, Union, get_args, get_origin -from typing import Callable as t_Callable -from typing import Dict as t_Dict # noqa: UP035 -from typing import Mapping as t_Mapping # noqa: UP035 +from typing import TYPE_CHECKING, Any, get_origin try: @@ -18,17 +13,30 @@ from docutils.parsers.rst.roles import set_classes from docutils.parsers.rst.states import Inliner, Struct from docutils.utils import SystemMessage, unescape -from sphinx_autodoc_typehints import format_annotation as _format_orig from scanpydoc import elegant_typehints if TYPE_CHECKING: + from collections.abc import Iterable, Sequence + from docutils.nodes import Node from sphinx.config import Config -def _format_full(annotation: type[Any], config: Config) -> str | None: +def typehints_formatter(annotation: type[Any], config: Config) -> str | None: + """Generate reStructuredText containing links to the types. + + Can be used as ``typehints_formatter`` for :mod:`sphinx_autodoc_typehints`, + to respect the ``qualname_overrides`` option. + + Args: + annotation: A type or class used as type annotation. + config: Sphinx config containing ``sphinx-autodoc-typehints``’s options. + + Returns: + reStructuredText describing the type + """ if inspect.isclass(annotation) and annotation.__module__ == "builtins": return None @@ -53,72 +61,6 @@ def _format_full(annotation: type[Any], config: Config) -> str | None: return None -def _format_terse(annotation: type[Any], config: Config) -> str: - origin = get_origin(annotation) - args = get_args(annotation) - tilde = "" if config.typehints_fully_qualified else "~" - fmt = partial(_format_terse, config=config) - - # display `Union[A, B]` as `A | B` - if origin in ({Union, UnionType} - {None}): - # Never use the `Optional` keyword in the displayed docs. - # Use `| None` instead, similar to other large numerical packages. - return " | ".join(map(fmt, args)) - - # do not show the arguments of Mapping - if origin in (Mapping, t_Mapping): - return f":py:class:`{tilde}collections.abc.Mapping`" - - # display dict as {k: v} - if origin in (dict, t_Dict) and len(args) == 2: # noqa: UP006, PLR2004 - k, v = args - return f"{{{fmt(k)}: {fmt(v)}}}" - - # display Callable[[a1, a2], r] as (a1, a2) -> r - if origin in (Callable, t_Callable) and len(args) == 2: # noqa: PLR2004 - params, ret = args - params = ["…"] if params is Ellipsis else map(fmt, params) - return f"({', '.join(params)}) → {fmt(ret)}" - - # display Literal as {'a', 'b', ...} - if origin is Literal: - return f"{{{', '.join(map(repr, args))}}}" - - return _format_full(annotation, config) or _format_orig(annotation, config) - - -def format_annotation(annotation: type[Any], config: Config) -> str | None: - """Generate reStructuredText containing links to the types. - - Unlike :func:`sphinx_autodoc_typehints.format_annotation`, - it tries to achieve a simpler style as seen in numeric packages like numpy. - - Args: - annotation: A type or class used as type annotation. - config: Sphinx config containing ``sphinx-autodoc-typehints``’s options. - - Returns: - reStructuredText describing the type - """ - curframe = inspect.currentframe() - calframe = inspect.getouterframes(curframe, 2) - if calframe[2].function in {"process_docstring", "_inject_signature"} or ( - calframe[2].function == "_inject_types_to_docstring" - and calframe[3].function == "process_docstring" - ): - return format_both(annotation, config) - # recursive use - return _format_full(annotation, config) - - -def format_both(annotation: type[Any], config: Config) -> str: - terse = _format_terse(annotation, config) - full = _format_full(annotation, config) or _format_orig(annotation, config) - if terse == full: - return terse - return f":annotation-terse:`{_escape(terse)}`\\ :annotation-full:`{_escape(full)}`" - - def _role_annot( # noqa: PLR0913 name: str, # noqa: ARG001 rawtext: str, @@ -148,10 +90,6 @@ def _role_annot( # noqa: PLR0913 return [node], messages -def _escape(rst: str) -> str: - return rst.replace("`", "\\`") - - def _unescape(rst: str) -> str: # IDK why the [ part is necessary. return unescape(rst).replace("\\`", "`").replace("[", "\\[") diff --git a/src/scanpydoc/elegant_typehints/_return_tuple.py b/src/scanpydoc/elegant_typehints/_return_tuple.py index e3fb1d2..6c1690c 100644 --- a/src/scanpydoc/elegant_typehints/_return_tuple.py +++ b/src/scanpydoc/elegant_typehints/_return_tuple.py @@ -12,7 +12,7 @@ except ImportError: UnionType = None -from ._formatting import format_both +from sphinx_autodoc_typehints import format_annotation if TYPE_CHECKING: @@ -71,7 +71,7 @@ def process_docstring( # noqa: PLR0913 idxs_ret_names = _get_idxs_ret_names(lines) if len(idxs_ret_names) == len(ret_types): for l, rt in zip(idxs_ret_names, ret_types): - typ = format_both(rt, app.config) + typ = format_annotation(rt, app.config) lines[l : l + 1] = [f"{lines[l]} : {typ}"] diff --git a/src/scanpydoc/elegant_typehints/static/typehints.css b/src/scanpydoc/elegant_typehints/static/typehints.css deleted file mode 100644 index 23c5302..0000000 --- a/src/scanpydoc/elegant_typehints/static/typehints.css +++ /dev/null @@ -1,6 +0,0 @@ -*:not(:hover) > .annotation.full { - display: none; -} -*:hover > .annotation.terse { - display: none; -} diff --git a/tests/test_elegant_typehints.py b/tests/test_elegant_typehints.py index fb9e6e3..9a0bcee 100644 --- a/tests/test_elegant_typehints.py +++ b/tests/test_elegant_typehints.py @@ -4,14 +4,12 @@ import inspect import re -import sys from collections.abc import Callable, Mapping from pathlib import Path from typing import ( TYPE_CHECKING, Any, AnyStr, - Literal, NoReturn, Optional, Union, @@ -20,13 +18,8 @@ import pytest import sphinx_autodoc_typehints as sat -from sphinx.testing.restructuredtext import parse -from scanpydoc.elegant_typehints._formatting import ( - _format_full, - _format_terse, - format_annotation, -) +from scanpydoc.elegant_typehints._formatting import typehints_formatter from scanpydoc.elegant_typehints._return_tuple import process_docstring @@ -34,7 +27,6 @@ from types import FunctionType, ModuleType from sphinx.application import Sphinx - from sphinx.config import Config @pytest.fixture() @@ -88,14 +80,30 @@ def test_app(app: Sphinx) -> None: def test_default(app: Sphinx) -> None: - assert format_annotation(str, app.config) is None + assert typehints_formatter(str, app.config) is None + + +def _escape_sat(rst: str) -> str: + rst = ( + rst.replace("\\", r"\\") + .replace("`", r"\`") + .replace(":", r"\:") + .replace("~", r"\~") + .replace(",", r"\,") + .replace("[", r"\[") + .replace("]", r"\]") + ) + return f":sphinx_autodoc_typehints_type:`{rst}`" def test_alternatives(process_doc: Callable[[FunctionType], list[str]]) -> None: def fn_test(s: str): # noqa: ANN202, ARG001 """:param s: Test""" - assert process_doc(fn_test) == [":type s: :py:class:`str`", ":param s: Test"] + assert process_doc(fn_test) == [ + f":type s: {_escape_sat(':py:class:`str`')}", + ":param s: Test", + ] def test_defaults_simple(process_doc: Callable[[FunctionType], list[str]]) -> None: @@ -106,11 +114,11 @@ def fn_test(s: str = "foo", n: None = None, i_: int = 1): # noqa: ANN202, ARG00 """ # noqa: D205 assert process_doc(fn_test) == [ - ":type s: :py:class:`str` (default: ``'foo'``)", + f":type s: {_escape_sat(':py:class:`str`')} (default: ``'foo'``)", ":param s: Test S", - ":type n: :py:obj:`None` (default: ``None``)", + f":type n: {_escape_sat(':py:obj:`None`')} (default: ``None``)", ":param n: Test N", - r":type i\_: :py:class:`int` (default: ``1``)", + rf":type i\_: {_escape_sat(':py:class:`int`')} (default: ``1``)", r":param i\_: Test I", ] @@ -119,89 +127,25 @@ def test_defaults_complex(process_doc: Callable[[FunctionType], list[str]]) -> N def fn_test(m: Mapping[str, int] = {}): # noqa: ANN202, ARG001 """:param m: Test M""" + expected = ( + r":py:class:`~collections.abc.Mapping`\ \[:py:class:`str`, :py:class:`int`]" + ) assert process_doc(fn_test) == [ - ":type m: " - r":annotation-terse:`:py:class:\`~collections.abc.Mapping\``\ " - r":annotation-full:`" - r":py:class:\`~collections.abc.Mapping\`\[:py:class:\`str\`, :py:class:\`int\`]" - "` (default: ``{}``)", + f":type m: {_escape_sat(expected)} (default: ``{{}}``)", ":param m: Test M", ] -def test_mapping(app: Sphinx) -> None: - assert ( - _format_terse(Mapping[str, Any], app.config) - == ":py:class:`~collections.abc.Mapping`" - ) - assert _format_full(Mapping[str, Any], app.config) is None - - -def test_dict(app: Sphinx) -> None: - assert _format_terse(dict[str, Any], app.config) == ( - "{:py:class:`str`: :py:data:`~typing.Any`}" - ) - - -@pytest.mark.parametrize( - ("annotation", "expected"), - [ - (Callable[..., Any], "(…) → :py:data:`~typing.Any`"), - ( - Callable[[str, int], None], - "(:py:class:`str`, :py:class:`int`) → :py:obj:`None`", - ), - ], -) -def test_callable_terse(app: Sphinx, annotation: type, expected: str) -> None: - assert _format_terse(annotation, app.config) == expected - - -def test_literal(app: Sphinx) -> None: - assert _format_terse(Literal["str", 1, None], app.config) == "{'str', 1, None}" - assert _format_full(Literal["str", 1, None], app.config) is None - - -@pytest.mark.skipif(sys.version_info < (3, 10), reason="Syntax only available on 3.10+") -def test_syntax_vs_typing(app: Sphinx) -> None: - u = eval("int | str") # noqa: PGH001, S307 - assert _format_terse(u, app.config) == ":py:class:`int` | :py:class:`str`" - assert _format_full(u, app.config) is None # this used to crash - - def test_qualname_overrides_class(app: Sphinx, testmod: ModuleType) -> None: assert testmod.Class.__module__ == "testmod" - assert _format_terse(testmod.Class, app.config) == ":py:class:`~test.Class`" + assert typehints_formatter(testmod.Class, app.config) == ":py:class:`~test.Class`" def test_qualname_overrides_exception(app: Sphinx, testmod: ModuleType) -> None: assert testmod.Excep.__module__ == "testmod" - assert _format_terse(testmod.Excep, app.config) == ":py:exc:`~test.Excep`" - + assert typehints_formatter(testmod.Excep, app.config) == ":py:exc:`~test.Excep`" -def test_qualname_overrides_recursive(app: Sphinx, testmod: ModuleType) -> None: - assert _format_terse(Union[testmod.Class, str], app.config) == ( - r":py:class:`~test.Class` | :py:class:`str`" - ) - assert _format_full(Union[testmod.Class, str], app.config) is None - - -def test_fully_qualified(app: Sphinx, testmod: ModuleType) -> None: - app.config.typehints_fully_qualified = True - assert _format_terse(Union[testmod.Class, str], app.config) == ( - r":py:class:`test.Class` | :py:class:`str`" - ) - assert _format_full(Union[testmod.Class, str], app.config) is None - -def test_classes_get_added(app: Sphinx) -> None: - doc = parse(app, r":annotation-full:`:py:class:\`str\``") - assert doc[0].tagname == "paragraph" - assert doc[0][0].tagname == "inline" - assert doc[0][0]["classes"] == ["annotation", "full"] - - -@pytest.mark.parametrize("formatter", [_format_terse, _format_full], ids="tf") # These guys aren’t listed as classes in Python’s intersphinx index: @pytest.mark.parametrize( "annotation", @@ -218,7 +162,6 @@ def test_classes_get_added(app: Sphinx) -> None: def test_typing_classes( app: Sphinx, annotation: type, - formatter: Callable[[type, Config], str], ) -> None: app.config.typehints_fully_qualified = True name = ( @@ -228,9 +171,7 @@ def test_typing_classes( # 3.6 _Any and _Union or annotation.__class__.__name__[1:] ) - if formatter is _format_terse and name in {"Union", "Callable"}: - pytest.skip("Tested elsewhere") - output = formatter(annotation, app.config) + output = typehints_formatter(annotation, app.config) assert output is None or output.startswith(f":py:data:`typing.{name}") @@ -332,10 +273,9 @@ class B: (Optional[tuple[str, int]], ":py:class:`str`"), ( tuple[Mapping[str, float], int], - r":annotation-terse:`:py:class:\`~collections.abc.Mapping\``\ " - r":annotation-full:`:py:class:\`~collections.abc.Mapping\`\[" - r":py:class:\`str\`, :py:class:\`float\`" - r"]`", + r":py:class:`~collections.abc.Mapping`\ \[" + ":py:class:`str`, :py:class:`float`" + "]", ), ], ids=["tuple", "Optional[Tuple]", "Complex"], @@ -351,31 +291,10 @@ def fn_test(): # noqa: ANN202 fn_test.__doc__ = docstring fn_test.__annotations__["return"] = return_ann - lines = [ - l - for l in process_doc(fn_test) - if not re.match("^:(rtype|param|annotation-(full|terse)):", l) - ] + lines = [l for l in process_doc(fn_test) if not re.match("^:(rtype|param):", l)] assert lines == [ f":return: foo : {foo_rendered}", " A foo!", " bar : :py:class:`int`", " A bar!", ] - - -def test_return_too_many(process_doc: Callable[[FunctionType], list[str]]) -> None: - def fn_test() -> tuple[int, str]: - """:return: foo - A foo! - bar - A bar! - baz - A baz! - """ # noqa: D205 - - assert not any( - "annotation-terse" in l - for l in process_doc(fn_test) - if not l.startswith(":rtype:") - )