Skip to content

6418 autodoc typehints description #7018

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

Merged
merged 3 commits into from
Jan 30, 2020

Conversation

tk0miya
Copy link
Member

@tk0miya tk0miya commented Jan 11, 2020

Feature or Bugfix

  • Feature

Purpose

Tasks

  • tests
  • documentation
  • tests with napoleon.

@svenevs
Copy link
Contributor

svenevs commented Jan 15, 2020

This is awesome, thank you so much for revisiting this! I have put some time on my calendar to take a closer look this weekend but wanted to ping you since some time had passed since you requested review.

The only thing that sticks out to me is how other domains could possibly take advantage of this event. Are there things missing from the call signature that other domains would want? Should the typehints extension fail if the domain is anything other than Python (given the intent to eventually merge into autodoc)? Or warn?

@tk0miya
Copy link
Member Author

tk0miya commented Jan 16, 2020

The type of domain and objtype are passed to event handler for object-description-transform. So event handlers can work only for arbitrary pair of domain and objtype. Actually, sphinx.ext.autodoc.typehints extension only runs only for python domain. So you don't worry about it breaks other domains.

@tk0miya tk0miya force-pushed the 6418_autodoc_typehints_description branch from 4d46668 to 67817ac Compare January 25, 2020 08:45
@tk0miya tk0miya added the type:enhancement enhance or introduce a new feature label Jan 25, 2020
@tk0miya tk0miya marked this pull request as ready for review January 25, 2020 08:55
@tk0miya tk0miya force-pushed the 6418_autodoc_typehints_description branch from 67817ac to 5397664 Compare January 25, 2020 08:56
@tk0miya
Copy link
Member Author

tk0miya commented Jan 25, 2020

I finished all my work. It's time to merge!

@svenevs
Copy link
Contributor

svenevs commented Jan 26, 2020

@tk0miya (-sensei!!!!!!!!) this is absolutely fantastic. You are a magician.

I was trying to modify napoleon but apparently it is not necessary for the bulk of this work.

Napoleon Overrides (docs needed)

I think this should very much stay as is and be considered a feature, I can document this pending return stuff below. If you manually annotate, this will override the injected :type: description.

def foo(arg: str) -> str:
    """
    Some doc.

    Parameters
    ----------
    arg : int
        This will get documented as ``int`` not ``str``.  Remove `` : int`` and it will be ``str``.

    Returns
    -------
    str
        I am looking into this one.  It is a slight problem.
    """

I think it would be good to document the new options in napoleon. I can do that / it does not need to block this PR, just putting down notes.

Napoleon Return Clauses

For return value, it would be nice if we could avoid manually specifying the return types.

  Return
  ------
+ A path is returned
- pathlib.Path
-     A path is returned.

This is the current design of napoleon (specify return type directly). I am trying to special-case this. It seems like it would be nice to have this as description for this PR, as well as generally (in general case, no :rtype: created).

I'm playing with this code here but wanted to send you a response while I continue looking into things.

Misc Build Caching Problem

When testing, I noticed that if you have autodoc_typehints = "description" and build html, and then change to autodoc_typehints = "signature" (or "none"), sphinx is convinced that nothing has changed and therefore does not regenerate anything. We currently must make clean and then make html to regenerate. Is this possible to fix, or should just be documented?


Thank you again so much for doing this, this is fantastic!

@svenevs
Copy link
Contributor

svenevs commented Jan 27, 2020

Update re: return clauses.

Google docstring is safe. I think. You can omit the return type. Though I can't actully figure out how to do "multiple-return-type" in the google format.

Numpy seems to have a somewhat irreconcilable problem. Apparently it is setup to enable multiple return types, so format is

Returns
--------
rtype_1
   rdesc_1
rtype_2
   rdesc_2

So this creates a conundrum. If we want to enable just writing the docstring directly under Returns without indentation (at level where rtype goes right now).

fields = self._consume_returns_section()

That field list is what matters. I played around with things but I think we have to add a new config var that says "only one return type for numpy" and then just dedent(...) the returns section as a whole.

  1. It is hard to detect "user wanted multi-return-type" vs "user wanted to use multiple paragraphs with auto rtype [this PR]". Forcing indentation under Returns does not change this (the different paragraphs get parsed as "return types" with empty description strings).

  2. If you let self._consume_reurns_section() happen, then try to do something hacky like re-join the fields, you lose nested structure. For example,

    Return
    ------
    A path is returned.  A long paragraph.
    It continues to the next line.
    
    This is a longer description.
    
    .. danger::
    
        Danger is among us.
    
        .. note::
    
            I'm still in the docstring!
    
    How fun.  To be in a docstring.

    You lose things if you let "normal" consumption happen, in this case the indentation before Danger is among us gets lost and therefore .. danger:: has no body (fail). Will investigate more as to how to do the "just dedent(...) the returns section", but wanted to share what I'm trying in case people have thoughts.

@tk0miya
Copy link
Member Author

tk0miya commented Jan 30, 2020

Thank you for investigation. That's very helpful to me. But as I commented at tk0miya#4, it would be better to merge this into 2.0. And let's discuss about napoleon in another PR.
I believe autodoc and napoleon get better with your support.
Thank you!

@tk0miya tk0miya merged commit 0f8a868 into sphinx-doc:2.0 Jan 30, 2020
@tk0miya tk0miya deleted the 6418_autodoc_typehints_description branch January 30, 2020 14:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 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.

3 participants