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

Close #2755: autodoc: Support type_comment style annotation #6984

Merged
merged 3 commits into from Jan 13, 2020

Conversation

tk0miya
Copy link
Member

@tk0miya tk0miya commented Jan 2, 2020

Feature or Bugfix

  • Feature

Purpose

@tk0miya tk0miya added type:enhancement enhance or introduce a new feature extensions:autodoc labels Jan 2, 2020
@tk0miya tk0miya added this to the 2.4.0 milestone Jan 2, 2020
@tk0miya tk0miya force-pushed the 2755_type_comment_support branch 2 times, most recently from f27444d to dac0e13 Compare January 2, 2020 15:50
@tk0miya tk0miya force-pushed the 2755_type_comment_support branch 2 times, most recently from 37517a9 to a5f3ca1 Compare January 11, 2020 12:25
@tk0miya tk0miya force-pushed the 2755_type_comment_support branch 4 times, most recently from 7095d52 to 9158d5e Compare January 11, 2020 13:23
Copy link
Member Author

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

It's time to merge. I'll merge this after minor fixes.

CHANGES Outdated Show resolved Hide resolved
sphinx/ext/autodoc/type_comment.py Show resolved Hide resolved
@tk0miya tk0miya merged commit ef81153 into sphinx-doc:2.0 Jan 13, 2020
@tk0miya tk0miya deleted the 2755_type_comment_support branch January 13, 2020 04:35
@tk0miya
Copy link
Member Author

tk0miya commented Jan 13, 2020

Merged. Thank you for reviewing! @shimizukawa @eric-wieser

@eric-wieser
Copy link
Contributor

What's the purpose of autodoc-before-process-signature here? Did you envisage any use case other than parsing type comments?

It seems like this code would become simpler if you just built the type comment knowledge into sphinx.util.inspect.signature.

@eric-wieser
Copy link
Contributor

Edit: Ah, I see it's to make it opt in. Maybe the extension should be called sphinx.ext.typecomments, and be hooked into at the inspect.signature level?

@tk0miya
Copy link
Member Author

tk0miya commented Mar 27, 2020

It was added to update the signature of function before processing. The use case of the event are 1) type_comments and 2) .pyi files (refs: #4824). I don't know there are any more use case for the event. But I thought it is simple to separate them from inspect.signature(). What do you think?

In addition, the sphinx.ext.autodoc.type_comment was young during 2.4.x. So I marked it as experimental, and separate it to an independent extension from autodoc not to load by default. But, now it becomes a formal feature since 3.0. So it is a part of autodoc and loaded by default.

@eric-wieser
Copy link
Contributor

But I thought it is simple to separate them from inspect.signature(). What do you think?

What I've found is that you seem to end up having to implement the internals of inspect.signature repeatedly, because every time it visits a function, you need to call the hook before looking at it.

See #7384 for an example of this.

Good point about the .pyi files, I hadn't thought about that. Although perhaps simpler would be a third party module that consumes a .pyi file and applies all the annotations in it, rather than only applying them when requested.

@tk0miya
Copy link
Member Author

tk0miya commented Mar 27, 2020

Indeed. The autodoc + type_comment + Sphinx's inspect module would be an enhanced version of inspect.signature(). It generates documents for functions (signatures) from living objects and source code. I considered the responsibility of inspect.signature() is to obtain a signature from a living object. And to parse type_comment from source code is not. I don't know this is a best design.
Personally, I don't like to create a complex function having multiple responsibilities. But I'm okay to change the design if your proposal is good :-)

Anyway, the event has been released as stable. So we need to keep it working for a while if we'll mark it deprecated.

About the .pyi files, I still don't take a look in detail. So I can't discuss it well. As you said, it might be better to hook them on loading a target module.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
extensions:autodoc type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants