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

Support PEP 695 and PEP 696 syntax in py:function and py:class directives. #11444

Merged
merged 25 commits into from
Jul 23, 2023

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented May 28, 2023

Add support for PEP 695 and PEP 696.

  • Generic classes can be documented with .. py:class:: using PEP 695 syntax:

    .. py:class:: Sequence[T]
  • Generic functions can be documented with .. py:function:: using PEP 695 syntax:

    .. py:function:: foo[T](x: T)
  • Default values for type bounds are supported.

Generic type aliases introduced in this PEP are not yet supported (and I don't know if we should).

Closes #11438.

Implementation details

Since PEP 696 is not yet accepted, it is not possible to use an AST-approach or to use the current implementation of signature_from_ast. The reason is that variadic positional and keyword arguments cannot have default values (a SyntaxError is raised when parsing the code), so I used a token-based approach.

In addition, PEP 695 specifications allow the type parameter bound to be any kind of expression, that is, P: int + int should be valid. Since the tokenizer consumes whitespace outside of literals, I implemented a small prettifier which automatically adds spaces around arithmetic operators (there is a test showing the different results).

Notes

I am opening a PR now because I don't know if there are other places that need attention and thus it would be good to let me know where other places need to be changed. Also, the current implementation tries to limit the changes on the writer's side and thus the type parameters list is handled the same way the parameters list is (so the multiline support is enabled for it as well).

@picnixz picnixz force-pushed the feature/11438-PEP-695-support branch from a8553f5 to bfb843b Compare May 28, 2023 17:31
@picnixz
Copy link
Member Author

picnixz commented May 28, 2023

@JelleZijlstra The following

.. py:class:: Sequence[T](Iterable[T])

.. py:function:: add[T](x: T, y: T)

.. py:function:: _add(x: T, y: T)

renders as

Screenshot_20230528_194013

Would it be acceptable ? or do you prefer that the type parameters are straight instead of in italics?

@JelleZijlstra
Copy link
Contributor

That looks great to me! I don't have strong feelings on whether to use italics. It makes sense to use the same style for type parameters as for annotations.

@picnixz
Copy link
Member Author

picnixz commented May 28, 2023

I tried to find some "complex" examples for this new syntax but I would like to know whether there is a way to generate test cases based on the AST Python grammar in such a way that we could detect corner cases more easily (I especially want to test the new regex which seems... well.. simple?).

As AA-Turner said, cpython would need to upgrade the minimal sphinx version if my PR is merged so you'll still need to stick with the old syntax (PEP 695 is really amazing btw).

@picnixz picnixz force-pushed the feature/11438-PEP-695-support branch from bfb843b to 685d1b1 Compare May 29, 2023 16:04
Copy link
Contributor

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thank you!

tests/test_domain_py.py Show resolved Hide resolved
@picnixz picnixz force-pushed the feature/11438-PEP-695-support branch from 685d1b1 to 882d9d8 Compare May 30, 2023 09:59
@picnixz picnixz changed the title Support PEP 695 syntax in py:function and py:class directives. Support PEP 695 and PEP 696 syntax in py:function and py:class directives. May 30, 2023
@picnixz
Copy link
Member Author

picnixz commented Jun 1, 2023

I remembered the LaTeX writer only today. But in order to make this PR work with LaTeX, a bit more work needs to be done (and this involves changing the macros). Since I haven't worked enough with the LaTeX core, I am not confident in implementing this.

@jfbu What would be the best approach in the LaTeX writer ? my idea was to essentially handle the type parameters list the same way the regular parameter list is handled but the delimiters '()' are hardcoded on the LaTeX side. So, my idea was to rewrite the macro for creating a function/class signature with an additional parameter that would take the type parameters to format. But then, the visitor must be updated and the flow will be a bit different (for now, it just ignores type parameters node, so it does not even render them).

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2023

@picnixz Will the type parameter list obey "one type per line", is this what you mean with "multiline support is enabled for it", and I suppose no new config variable is related to this (I did not check too much for current lack of availability your work so far). As to the hard-coded things in the current sphinxlatexobjects.sty file such as \sphinxcode{(} converting it into some much better \sphinxarglistopenparen and maybe have some \sphinxtypelistopenbracket will be good. Maybe we should use rather\sphinxarglistopendelim, \sphinxtypelistopendelim. And perhaps use or not legacy \pysig prefix for such commands. As far as I can tell from cursory reading of the PEP 695 examples, it looks as if the type list could be handled by extending the LaTeX writer mark-up with \pysigline, \pysiglinewithargsret to feed them with thean optional (in the LaTeX sense) argument, i.e. the LaTeX declarations in sphinxlatexobjects.sty would then have to be extended to \newcommand{\pysigline}[2][] in place of \newcommand{\pysigline}[1] with some test of emptiness of #1 now standing for type list whereas arg list will now be a #2, or, and perhaps that is better, let's simply duplicate LaTeX macros \pysigline into an unchanged copy and a \pysiglinewithtypes extension and thus also \pysiglinewithargsret and \pysigwithonelineperarg will each need duplication and adaptations. Note that we are now using wrapping each argument into a \sphinxparam, and we should as well wrap each type into a \sphinxtype. I am not fully available these days, but will try to be reactive if you get something started. The LaTeX writer can be in first approach simply extended to imitate output mark-up from arg list to type list, with some LaTeX macros which can be defined to do nothing (gobble their arguments), and once this canvas is in place, I can help into sphinxlatexobjects.sty write-up if you have some hesitancy about it.

@picnixz
Copy link
Member Author

picnixz commented Jun 3, 2023

Will the type parameter list obey "one type per line", is this what you mean with "multiline support is enabled for it", and I suppose no new config variable is related to this (I did not check too much for current lack of availability your work so far).

Yes, the idea is to have the same behaviour for the tplist as for the arglist. Essentially, I don't want to separate the two mechanisms for now so currently the writers do exactly the same for a type parameters list node or a parameters list node.

As to the hard-coded things in the current sphinxlatexobjects.sty file such as \sphinxcode{(} converting it into some much better \sphinxarglistopenparen and maybe have some \sphinxtypelistopenbracket will be good.

Good idea. Since only ( and [ are to be used, I will change a bit the implementation and go for implementing an explicit visitor method for the new nodes.

It looks as if the type list could be handled by extending the LaTeX writer mark-up with \pysigline, \pysiglinewithargsret to feed them with thean optional (in the LaTeX sense) argument, i.e. the LaTeX declarations in sphinxlatexobjects.sty would then have to be extended to \newcommand{\pysigline}[2][] in place of \newcommand{\pysigline}[1]

That was my idea ! but since I never touched this part of the code (and since it is barely commented) I didn't want to modify anything. Also, it's quite difficult to navigate through LaTeX code in general since autocompletion features are not well-supported =/.

Anyway, the best idea is simply to assume that we have now two lists to render separately but with the same idea and both of them support the same functionalities, except that their delimiters are different.


Concerning the Python implementation, it currently does the following:

  • Find the first desc_parameterlist node and handle it directly.
  • When handling it, the correct macros are written.

However with the desc_tparameterlist, we would do it twice and I am unsure on how to properly do it without having some dangling macros (maybe a straight implementation would be sufficient but it entirely depends on how we actually implement the LaTeX macros).

I think this part of the code was essentially modified by @TLouf in #11011 so maybe they may have some insight to share. I also need to update the other writers (and test them actually), especially the manpage and texinfo writers which I actually haven't tested or even visually checked at all.

Other remarks

There were more parts where the code needed to be modified because I remembered the autodoc as well. From Python 3.12 and onwards, autodocumented generic classes are (for now) not supported in the sense that the type parameters list is simply ignored (and not even stored). It is only matched but another PR needs to address this.

Final remarks

I am not fully available these days, but will try to be reactive if you get something started

I have upcoming conferences so I need to prepare the presentations and attend the conferences, so I won't be available from mid-June to mid-July. After that, I'll have some time in August but then again I'll be travelling from late-August to mid-September. I still think I'll manage to finish things before leaving but I'll ping you when I am stuck, so thank you in advance.

@picnixz
Copy link
Member Author

picnixz commented Jun 3, 2023

With the latest implementation, this is how LaTeX and HTML are rendered. I eventually decided to split the multi-line support as follows:

  • multi-line support is enabled for the type parameters list if the length of the signature deprived of its argument list (and return annotation) exceeds the expected length. It can be forcibly disabled using the single-line-type-parameter-list directive option.

  • multi-line support is enabled for the arguments list if the length of the signature deprived of its type parameter list exceeds the expected length or if single-line-parameter-list is specified (this was done in ENH: enable multiline signatures when too long. #11011).

I kinda visually tested the "unusal writers" but we'll probably need to add tests at some point (although I don't think that there are a lot of projects using the texinfo or the manpage with Python code).

Example

.. py:function:: foo(a, b, c)
                 foo(very_long_1, very_long_2, very_long_3)
                 foo[X, Y, Z](a, b, c)
                 foo[X, Y, Z](very_long_1, very_long_2, very_long_3)
                 foo[VERY_LONG_1, VERY_LONG_2, VERY_LONG_3](a, b, c)
                 foo[VERY_LONG_1, VERY_LONG_2, VERY_LONG_3](very_long_1, very_long_2, very_long_3)

No multi-line support

HTML

Screenshot_20230603_183105

LaTeX

Screenshot_20230603_182936

With multi-line support

HTML

Screenshot_20230603_182846

LaTeX

Screenshot_20230603_182731

@picnixz picnixz force-pushed the feature/11438-PEP-695-support branch from b212fb4 to e28c44d Compare June 3, 2023 16:54
@picnixz
Copy link
Member Author

picnixz commented Jun 3, 2023

@jfbu If you want to have a look, I ended up implementing a c/c of what was already written. If possible, I'd like you to review these parts (whenever you're available) and tell me whether some parts are superfluous or not (I am not entirely sure that all my \strut are needed).

Also, I purposely duplicated the different implementations so that we can, in a second phase, refactor them more easily. Technically speaking, it should be possible to have one single macro doing all the work or we could split some "same" functionalities as private macros and then use them directly.

Copy link
Contributor

@jfbu jfbu left a comment

Choose a reason for hiding this comment

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

I did a very superficial look only to check LaTeX macros added to support this feature, and this looks good to me. In future we should think into replacing \sphinxcode{(} et al. into better abstraction such as \sphinxarglistopen or \sphinxarglistleftdelimiter or whatever.

sphinx/texinputs/sphinxlatexobjects.sty Outdated Show resolved Hide resolved
@picnixz picnixz force-pushed the feature/11438-PEP-695-support branch from e58ecb9 to a924d4a Compare June 6, 2023 13:58
@AA-Turner AA-Turner merged commit 480630c into sphinx-doc:master Jul 23, 2023
@AA-Turner
Copy link
Member

@picnixz -- I've merged now as a base implementation, any improvements we need to make shouldn't hold up the feature -- thank you for your great work on this!

A

@picnixz picnixz deleted the feature/11438-PEP-695-support branch July 24, 2023 07:28
@jfbu
Copy link
Contributor

jfbu commented Jul 24, 2023

@picnixz @AA-Turner good this was merged; sorry I am not much available currently for (LaTeX) maintenance

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support PEP 695 syntax for class definitions
4 participants