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

enable removal of type hints from autodoc #6361

Closed
svenevs opened this issue May 13, 2019 · 9 comments
Closed

enable removal of type hints from autodoc #6361

svenevs opened this issue May 13, 2019 · 9 comments
Labels
extensions:autodoc type:enhancement enhance or introduce a new feature
Milestone

Comments

@svenevs
Copy link
Contributor

svenevs commented May 13, 2019

The proposed #6353 solution to #6311 presents an interesting opportunity to consistently and conveniently enable autodoc users to remove type hints. This can be achieved in one of two ways

  1. An autodoc config variable such as autodoc_remove_type_hints or something, which globally removes type hints from signatures. And/or
  2. An additional parameter to "autodoc-process-signature" event that is the documenter itself.

Type hints are great, but currently some of my signatures become very confused in the docs making it hard to understand.

Describe the solution you'd like

If we make the documenter available, this becomes quite simple. A breaking change would be to just tack a self on at the end of the event. I took a look around, but recovering the documenter from the information currently provided to "autodoc-process-signature" is not easy to do.

result = self.env.events.emit_firstresult('autodoc-process-signature',
self.objtype, self.fullname,
self.object, self.options, args, retann)

--- a/sphinx/ext/autodoc/__init__.py
+++ b/sphinx/ext/autodoc/__init__.py
@@ -413,7 +413,8 @@ class Documenter:
 
         result = self.env.events.emit_firstresult('autodoc-process-signature',
                                                   self.objtype, self.fullname,
-                                                  self.object, self.options, args, retann)
+                                                  self.object, self.options, args, retann,
+                                                  self)
         if result:
             args, retann = result

Giving the documenter (self) directly gives us a very clean way to remove type hints:

# NOTE: only valid with #6353 (added show_annotation ability)
def autodoc_process_signature(app, what, name, obj, options, signature, return_annotation, documenter):
    # We are only modifying functions / methods.  Return None signals no change.
    if what not in {"function", "method"}:
        return None

    try:
        return documenter.format_args(show_annotation=False), return_annotation
    except:
        return None

Considerations

Adding an additional parameter is a breaking change.

TypeError: autodoc_process_signature() takes 7 positional arguments but 8 were given

So the implementation should call with 8 and fallback to calling with 7, issuing a warning that 8 are required now (?).

I'm happy to implement this with some discussion. For example, with (1) adding a global option, would it be better to do something like

autodoc_remove_type_hints = [
    "function", "method"
]

so that it's only done for specific types? To be honest, I'd be thrilled with just (2) and add documentation showing users how to remove type hints if they want since it's very little code on their part, and trying to autopopulate the proposed **kwargs from #6353 may add a lot of complexity for not very much benefit.

@tk0miya
Copy link
Member

tk0miya commented May 14, 2019

As you said, 2) breaks compatibility of extensions. So I feel -1 for the option.

On the other hand, 1) is better idea. But I think removal is not best way because it loses type information from document. It would be much better if we can convert them into info field list.
http://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists

I believe it is best way to represent anotations in other way.

@svenevs
Copy link
Contributor Author

svenevs commented May 15, 2019

Ok. I opened #6374 as an example. It's not a "breaking" change as in hard-break, it's something that we can start warning about now and just fallback on the original behavior.

it loses type information from document.

This is the goal (for me). Complex type hints make it much harder to read, and I document my types using numpydoc style with napolean :)

If #6374 does not look ideal for you that is OK (I'm already monkeypatching that in there so I have a solution currently). But I don't have a lot of confidence in my abilities to actually do option (1) correctly...Just putting the PR up in case maybe I didn't explain breaking change well, it's a "soft break" until 4.x.

Edit: for clarity. When you say convert them into info field list, how would the types be removed in the end? I don't understand why converting to info field list helps this.

@tk0miya
Copy link
Member

tk0miya commented May 19, 2019

Edit: for clarity. When you say convert them into info field list, how would the types be removed in the end? I don't understand why converting to info field list helps this.

In my thought, all annotations on signatures are removed after conversion.

At present, Sphinx tries to render type annotations as is. But it is hard to read when its parameters are complex (see #5868). As you said, it is better to describe the types in contents area. But I think we should not write them on docstring manually because they are duplicated. It is annoying to me.

In my thought, all annotations are converted into contents as info field lists and remove them all from signature.

For example, the following python function will be converted as next reST markup.

def hello(name: str) -> str:
    """blah blah blah

    :param name: Your name
    :returns: greeting message
    """
.. py:function: hello(name)

    blah blah blah

    :param name: Your name
    :type name: str
    :returns: greeting message
    :rtype: str

@svenevs
Copy link
Contributor Author

svenevs commented May 19, 2019

I completely agree. I resurveyed all of the options recently and ultimately decided to just move on with a compromise:

  • Use numpy style (I find it the most readable in code, which I look at as often as rendered docs). This requires I duplicate the type hints (once in the signature, once in the docstring).
  • Find a way to remove them from the signature (this issue).

My ultimate plan was to do what you are suggesting, only specifically for Napoleon extension and numpy style.

So long term if we have a way to enable this for the :param: / :type: style then ideally we could find a way to generalize or reuse part of this for napoleon. This is a pretty big goal, but I think the community at large would be quite pleased. It's something that, if users are currently adding types in their docstring, would be as simple as once this feature is released, users can just delete the type components from their docstring.

As such the hacky PR I proposed is kind of negative progress on that front. I'll close it.

I won't be able to make much headway on this for at least a month, but it's definitely something I want to help enable long term :)

@tk0miya
Copy link
Member

tk0miya commented May 25, 2019

@shimizukawa @svenevs What do you think this setting?

autodoc_typehints = "signature" | "description" | "none"

* "signature": Show typehints as signature (as is). This is default choice.
* "description": Show typehints as object description (info field list or napoleon (if possible)).
* "none": Do not show typehints.

If agreed, I will implement "none" option at first. And will implement "description" option in future.

@svenevs
Copy link
Contributor Author

svenevs commented May 25, 2019

This sounds good to me. Please CC me on PRs to test!

When description time comes I think info field list will be more straightforward. I don't know the Sphinx internals very well but can learn from potential approach you take. I will then look at napoleon and see what I can find out :)

@tk0miya
Copy link
Member

tk0miya commented May 25, 2019

It seems @shimizukawa also likes it (see reaction). Let's go forward :-)

@tk0miya
Copy link
Member

tk0miya commented May 25, 2019

I just post #6361 for autodoc_typehints = 'none'.

I don't know the Sphinx internals very well but can learn from potential approach you take.

I don't have good idea to implement description. Inside sphinx, info field list is represented as just user content (like paragraph, table and so on). It is hard to merge typehints systematically. To support napoleon also, we need to add new mechanism (framework?) to merge them to user contents. I'll consider that later.

tk0miya added a commit to tk0miya/sphinx that referenced this issue May 25, 2019
tk0miya added a commit that referenced this issue May 31, 2019
Close #6361: autodoc: Add autodoc_typehints to suppress typehints from signature
@tk0miya
Copy link
Member

tk0miya commented May 31, 2019

Done in #6397.
Remaining works will be discussed in #6418 (thank you for filing!)
Closing.

@tk0miya tk0miya closed this as completed May 31, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 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

No branches or pull requests

2 participants