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

Add support for a Python decorator role #13105

Open
erlend-aasland opened this issue Nov 6, 2024 · 5 comments
Open

Add support for a Python decorator role #13105

erlend-aasland opened this issue Nov 6, 2024 · 5 comments
Labels
python Pull requests that update Python code type:proposal a feature suggestion

Comments

@erlend-aasland
Copy link

See previous discussion over at the CPython repo:

The decorator (:py:deco:) role would render the function/method reference with a prefixed @.

@timhoffm
Copy link
Contributor

timhoffm commented Nov 6, 2024

Just as a note, https://www.sphinx-doc.org/en/master/usage/domains/python.html#directive-py-decorator states

There is no py:deco role to link to a decorator that is marked up with this directive; rather, use the py:func role.

So that should be updated when the role gets implemented.

@jayaddison jayaddison added the python Pull requests that update Python code label Nov 8, 2024
@electric-coder
Copy link

electric-coder commented Nov 8, 2024

@erlend-aasland from the glossary:

decorator

A function returning another function

So syntactically there's nothing wrong with the existing roles, per se. It was a conservative choice to not further specialize the roles (it would arguably pollute the domain by adding an extra name - if such addition isn't motivated by a clear need and were nothing but an alias).

A couple of concerns arise from your original reasoning:

sometimes we want to explicitly refer to a decorator as a function or the other way around.

Generally py:meth: and py:func: aren't interchangeable - if you try using the wrong role the cross-reference won't resolve. This begs the question: you can decorate bound and unbound methods (and also classes for that matter), I'm not sure it's consistent within Sphinx (I have a hard time thinking of a comparable case) to allow two different roles to resolve to the same domain declaration (apart from the catch-all py:obj:).

Then @AlexWaygood (an Enum afficionado like me) makes this comment:

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.

The inclusion of the ending () is controlled by a Sphinx-wide configuration add_function_parentheses so I'm unsure what's being proposed here? Do you want to special case the decorators to override the Sphinx-wide config? Or do you want to add an extra configuration for the proposed py:deco: role? And the same can be asked if a leading @ is supposed to be included verbatim in the cross-reference, or if that is left up to the author...


  • I'll make an argument in favor of the proposed role: It would allow full-text search for py:deco: and that's a plus if trying to find all occurrences of decorator cross-references.

But aside from the above point I still fail to see a clear need, going back to your original argument:

There's already decorator and decoratormethod directives for signatures.

Yes those are domain declarations that can require specialized syntax, but the role used in cross-referencing may dispense specialization and I fail to see proof for its clear-cut need (apart from facilitating specialized full-text search).

Thus this argument by @CAM-Gerlach hasn't shown any concrete examples supporting it:

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


@timhoffm have there been previous FRs about this in the repo (that were closed for some reason, or that remain open)? Now would be the time to reexamine previous discussions about this.


So is this the only aim?:

The decorator (:py:deco:) role would render the function/method reference with a prefixed @.

This proposes an extra role with the sole purpose of shortening the syntax. Allowing :deco:`your_decorator` to be rendered as @your_decorator thus dispensing the need to write the more verbose :func:`@your_decorator <your_decorator>` to obtain the same result; but at the cost of introducing an extra role, and necessarily an extra Sphinx-wide config (not all authors will want the @ in their docs) and introducing the additional problem/exception of how 2 roles resolves to the same declaration.

The same result could be obtained by a single Sphinx-wide config that cosmetically prepends the @ to every :func: that's a decorator (the same as appending () to callables) - resolving on type can be explicit enough. (This is arguably more economical). The case of selectively including the @ in some cases but not in others introduces a problem of inconsistency, that in turn can be solved by selectivity writing the cross-reference when needed, so there's no concrete need of introducing an extra role where a single Sphinx-config suffices - because the role doesn't solve the selectivity problem of introducing @, or not, except by changing back to :func: which overall introduces more inconsistency that the proposed role does not resolve, not even in the slightest.

@erlend-aasland
Copy link
Author

Yes, apart from being easy to grep for, my initial thought was syntactic sugar: mark up references to decorators using :deco:`my_deco` instead of :func:`@my_deco <my_deco>` . My original plan was to implement this as an "extension" kept in the CPython repo, however @AA-Turner suggested I raise it as a feature request here instead. I totally understand if you don't want to pollute Sphinx with this feature, and I'm fine with closing this as wont-implement.

Now, my Sphinx knowledge is extremely shallow, and I never had the need to experiment with custom roles or directives; this combined with the lack of spare time to set aside for such a pet project lead to my draft PR growing stale pretty quick.

@electric-coder
Copy link

electric-coder commented Nov 8, 2024

my initial thought was syntactic sugar

My love affair with Python is rooted on syntactic sugar. But I'm thinking this kind of FR needs a comparison matrix showing syntactic pros/cons + Sphinx-configs - there are several combined factors at play.

I totally understand if you don't want to pollute Sphinx with this feature

Don't get me wrong, as an experienced Sphinx user I think it's best to be skeptical because many FRs seem nice at first glance but not on further reflection.

however @AA-Turner suggested I raise it as a feature request here instead

Scrutiny&discussion from the wider community is the way to go! I'll try to whoop up that pro/con matrix when I can.

@AA-Turner
Copy link
Member

I don't think there's a risk of pollution, whilst decorators are functions, they're semantically distinct enough for me. Note that we don't just have one 'callable' role, but split into individual types.

A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python Pull requests that update Python code type:proposal a feature suggestion
Projects
None yet
Development

No branches or pull requests

5 participants