search

sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.46k stars 2.1k forks source link

automatic type inference for napoleon numpy style #7077

Open svenevs opened 4 years ago

svenevs commented 4 years ago

CC: @tk0miya @rgommers

Refs:

This issue tracks two items:

def func(arg: int) -> str:
    """
    ...

    Returns
    -------
    str
        The style mandates the return-type be specified.  I would like to relax this.
    """

I was able to produce a patch that results in the Returns clause being allowed to document only one return type

Returns
-------
Some description of the thing.

https://github.com/numpy/numpydoc/issues/251#issuecomment-579753725

First impression: it may be useful for some packages, but I think we would recommend to not do this. The reason is that many users read docstrings in a terminal (e.g. IPython), so reducing the info in the docstring itself and letting Sphinx put it into html docs sounds sub-optimal.

The help(some_thing) string includes type annotations. Users choosing to adopt napoleon_numpy_returns_no_rtype do so because they already have a return-type annotated in the python function itself. Notably the new functionality introduced in https://github.com/sphinx-doc/sphinx/pull/7018 is only available for pure python code (I understand NumPy won't be adopting this!).

I understand if NumPy doesn't want a boolean triggered bastardization of the Returns clause. I can maintain an extension instead if that's the decision. I just feel like the changeset is so small, and I also believe (but have no metric) there is demand for this. I like the NumPy style so much that I willingly chose it even though I knew I'd have to duplicate type info, but we are now at the point where that is no longer necessary :slightly_smiling_face:

tk0miya commented 4 years ago

Thank you for opening issue and suggest for numpy project!

@tk0miya if I understand correctly, you don't want to merge docs on this until it lands in 3.x?

Yes, I feel typehints extension is still young. That is reason why I implemented it as experimental and an additional extension for autodoc. So I think it would be better to describe it only inside its document. http://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#generating-documents-from-type-annotations

About "Returns" type: TBH, I don't know that is it better to add a such option to napoleon. I worried about the option might bring separation to numpydoc. After Sphinx adopted it, some of users will write incorrect and broken numpy-style docstrings (very similar to original, but different).

So it would be very good if we can get agreed for new annotation. I understand your idea is very worthy. But I also admit help(some_thing) is also well used in console and jupyter notebook...

svenevs commented 4 years ago

@rgommers have you given this any additional consideration? Are there any other options? If I package it externally I have to monkeypatch napoleon, which I'm OK with but it's not exactly ideal :wink:

felix-hilden commented 4 years ago

Thank you for considering this! Here to express my support. Autotypehints and Napoleon are a clean and powerful combo. I'd hate to resort to using a bare :returns: to avoid duplication.

I also posted a comment about the effect on terminal docstrings on the numpydoc issue. Maybe I've misunderstood something, but as far as I can tell, return type hints (def thing(...) -> int) do show in help(thing). But I recognise that aligning with numpydoc is important.

Also, this seems related to #5887, although its rationale is a bit different.

tristanlatr commented 3 years ago

Hello, I have something working on my end that allows developer to specify the type when they actually need only.

So all of the following will work as expected:

Returns
-------
str
Returns
-------
str
    Description of return value
Returns
-------
Description of return value
Returns
-------
List[str]
    Description of return value
    Note
    ----
    Nested markup works
Returns
-------
list of str

Nested returns clauses inside methods clauses works also as expected

svenevs commented 3 years ago

Sorry for being MIA here. I wrote a hack a while ago but never got around to publishing it: https://github.com/svenevs/elba

The numpy people don't want this change in sphinx so the simplest possible monkey patch seems like a good approach. I can release that officially, pending discussion. I just want to avoid fragmentation, I think it is healthiest for everybody who wants this feature to converge on the same solution. I can also make it so that if elba is in the extension list that's an implicit default to this behavior.

I stopped before releasing because I wasn't sure how to accommodate mixed needs, e.g. is there are a couple of places where a user wants the old behavior. Some kind of list of function names?

As @tristanlatr has probably noticed, trying to accommodate all options is really tricky. I ultimately concluded that it's not possible given the constraints...

Let me know how people want to proceed, I'm for solving this once and for all in whatever way makes sense!

tristanlatr commented 3 years ago

e.g. is there are a couple of places where a user wants the old behaviour

Yes I think that keeping both syntax an option is the best to solve this issue.

I ultimately concluded that it's not possible given the constraints...

I think that you're right, it's not possible to have the perfect solution because of how numpy syntax is designed. This is why, it should be an option to disable manually, like napoleon_numpy_strict_returns=False

Anyway, right now people really need this to work with type annotations becoming more and more popular. The numpy people could very well change their mind after seeing the growing need for this feature.

I can open a PR if you like.

tk0miya commented 3 years ago

My opinion has not been changed since I commented last time. No reason to support the "rejected" syntax of numpydoc officially in Sphinx.

TBH, I don't know that is it better to add a such option to napoleon. I worried about the option might bring separation to numpydoc. After Sphinx adopted it, some of users will write incorrect and broken numpy-style docstrings (very similar to original, but different).

So it would be very good if we can get agreed for new annotation. I understand your idea is very worthy. But I also admit help(some_thing) is also well used in console and jupyter notebook... https://github.com/sphinx-doc/sphinx/issues/7077#issuecomment-580811850

rgommers commented 3 years ago

I had another look at it, and commented in https://github.com/numpy/numpydoc/issues/251#issuecomment-766178060. I'm much more confident in our rejection now, because for non-trivial code bases annotations are definitely not going to look like what you want users to see.

svenevs commented 3 years ago

Sounds good to me, I definitely agree on the general case. Type hints get pretty involved and once you're outside the py domain it gets really complicated!

Thanks for weighing in, I'll package elba for pure python users and update the docs on how to bypass the Union / Optional thing you mentioned. For parameters just put it in the docstring like normal (after the :, the docstring will supersede the signature), for Returns I think you just need to sneak in an :rtype: but will also look into allowing bypasses for the old style. Will close this issue once it's released. Anybody interested in comaintaining it please let me know, I don't expect this package to need much attention though.

It's also worth mentioning for future readers that you can avoid all of this is you adopt the Google style docstrings.

tristanlatr commented 3 years ago

Hi @svenevs , I've been developing a reliable way to allow the users to specify the types when dealing with non-trivial code bases annotations as well as skip it and only write free form text when the annotions are becoming redundant.

You can read the tests from here: https://github.com/tristanlatr/pydoctor/blob/napoleon/pydoctor/test/test_napoleon_docstring.py#L2315

I've forked the napoleon extension as a whole but it should be also possible to offer that feature by patching the _consume_field methods and associates.

psarka commented 2 years ago

Just voicing an opinion - I work with trivial code bases, my types are understandable, and having to duplicate return type is annoying. Would love to have an option to skip them as in the @tristanlatr example.

tristanlatr commented 2 years ago

Our Napoleon fork code is available there: https://github.com/twisted/pydoctor/tree/master/pydoctor/napoleon if anyone wants to provide the same kind of features for Sphinx :)

znichollscr commented 1 year ago

It's in a downstream repo, but this arguably gives a way to solve the issue https://github.com/tox-dev/sphinx-autodoc-typehints/pull/311