cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Docs: make it easier to reference and markup decorators

Open erlend-aasland opened this issue 2 years ago • 5 comments

Originally posted by @erlend-aasland in https://github.com/python/cpython/pull/105792#discussion_r1230584709

It would be nice if we had a "decorator role" that used the Python function/method markup, but instead of adding trailing parens, added a prefixing '@': :decorator:`dataclasses.dataclass` => @dataclasses.dataclass.

There's already decorator and decoratormethod directives for signatures. Perhaps we can exploit these so the :py:meth: and py:func: roles adapt the format according to what directive is used. A role could be a nice supplement, though; sometimes we want to explicitly refer to a decorator as a function or the other way around.

erlend-aasland avatar Jun 15 '23 08:06 erlend-aasland

https://github.com/python/cpython/blob/da911a6b226ca47cc15088d800b575e19a731f1c/Doc/tools/extensions/pyspecific.py#L713-L714

https://github.com/python/cpython/blob/da911a6b226ca47cc15088d800b575e19a731f1c/Doc/tools/extensions/pyspecific.py#L345-L356

erlend-aasland avatar Jun 15 '23 08:06 erlend-aasland

I really like the idea of having a py:deco: role for marking up decorators. There's lots of docs PRs I would have used this on in the past; I think it would be very useful. When discussing decorators, I usually want the @ prefixed before the decorator name, and I usually don't want the () after the decorator name that the py:func: role adds.

I'm not sure how I feel about py:func: and py:meth: having different rendering depending on whether the function/method being linked to is a decorator or not. That feels like it might be somewhat surprising behaviour for people new to it?

AlexWaygood avatar Jun 15 '23 08:06 AlexWaygood

A :py:deco: role seems most reasonable to me, as it is most consistent with how other multiple roles that mark up and style objects from a single namespace differently depending on the intended semantics operate, and it would also likely be a lot simpler, more robust and more efficient to implement than trying to monkeypatch the :py:func: and :py:meth: roles. It could also potentially get pulled into upstream Sphinx, if @AA-Turner is interested.

To note, Sphinx provides built-in enhanced versions of those two directives, so we could probably dump the custom overrides and switched to the fuller-featured, standard versions instead. It wouldn't really change anything either way with a :py:deco: role, though, since the lookup is ultimately the same.

CAM-Gerlach avatar Jun 15 '23 08:06 CAM-Gerlach

I'm not sure how I feel about py:func: and py:meth: having different rendering depending on whether the function/method being linked to is a decorator or not. That feels like it might be somewhat surprising behaviour for people new to it?

I agree. Explicit is better than implicit.

erlend-aasland avatar Jun 15 '23 08:06 erlend-aasland

Implementation-wise, it could be as simple as app.add_role_to_domain(domain="py", name="deco", role=sphinx.domains.python.PyXRefRole) if you don't need custom display behavior (e.g. prepending a @, or not appending ()).

If you do want that, you should be able to just subclass PyXRefRole, wrap process_link to prepend a @ to the returned title (for instance), and then just pass your subclassed PyXRefRole instead of the Sphinx one.

Of course, this doesn't actually check that the target is actually a decorator, but neither do the other :py: roles generally warn if their object types don't match (as they are all stored in the same index). You could implement something like that by modifying the custom decorator directive to add/store some sort of indicator metadata or a list of decorators, and then in process_link check against that list and emit a Sphinx warning if it doesn't match, but not required for an initial implementation.

CAM-Gerlach avatar Mar 20 '24 19:03 CAM-Gerlach

I started updating the documentation to make use of the :py:deco: role.

How should we reference objects such as classmethod, which is documented as a decorator, but is a class at runtime? There are places where it makes sense to use :deco:`classmethod` as classmethod is referenced as a decorator, but others where it is explicitly mentioned as being a type, e.g.:

https://github.com/python/cpython/blob/f81990024554a75e2ab31133a72d9f0954690435/Doc/library/abc.rst?plain=1#L248-L249

In this case, I'd say we could use :deco:`classmethod <classmethod>`, so that it renders as classmethod?

Similarly, how should @dataclass be referenced here?

https://github.com/python/cpython/blob/f81990024554a75e2ab31133a72d9f0954690435/Doc/howto/enum.rst?plain=1#L486-L504

Viicos avatar Mar 18 '25 20:03 Viicos

@AA-Turner, any reason this was closed? https://github.com/python/cpython/issues/131426 was closed pointing to this one as the tracking issue.

Viicos avatar Apr 17 '25 07:04 Viicos

I didn't see any open PRs linking to this, and the main work seemed done.

A

AA-Turner avatar Apr 17 '25 12:04 AA-Turner

I have pending work locally, but wanted to get feedback on https://github.com/python/cpython/issues/105812#issuecomment-2734666140 before continuing.

Viicos avatar Apr 17 '25 12:04 Viicos

I think it's fine to use either the class or deco role when referring to a decorator in the context of it being a class. I think both roles will generate the same cross-reference link.

A

AA-Turner avatar Apr 17 '25 13:04 AA-Turner

https://github.com/python/cpython/pull/139598 replaces some occurrences. To be checked still:

  • contextmanager
  • asynccontextmanager
  • dataclass
  • property.getter/setter
  • functools.total_ordering
  • functools.singledispatch
  • functools.wraps
  • reprlib.recursive_repr
  • Decorators in the test module (most likely not a lot of references)
  • typing.runtime_checkable
  • typing.dataclass_transform
  • typing.overload
  • typing.final
  • typing.no_type_check
  • typing.no_type_check_decorator
  • typing.override
  • typing.type_check_only
  • unittest.skip
  • unittest.skipIf
  • unittest.skipUnless
  • unittest.expectedFailure
  • warnings.deprecated

Viicos avatar Oct 05 '25 18:10 Viicos