-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
preprocessing numpy types #7690
preprocessing numpy types #7690
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your idea is great! LGTM!
gentle ping, @tk0miya. As a summary, this is what I think still needs to be answered / done (see also above):
|
gentle ping, @tk0miya |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for very very late response. I'm back from the maintenance of 3.1.x bug fixes now.
what is the default role used by the :type: field?
By default, :class:
role is used. As an exception, :obj:
is used for None
value.
how do I add location information to the warnings?
Unfortunately there no good way. The autodoc-process-docstring
that napoleon uses does not send a location of the docstring. It might be able to use self._obj
to obtain the name of the docstring owner.
how do I modify the test so that I can verify that a malformed value set emits the warning (something like self.assertWarnsRegex or pytest.warns that works with the logger)?
Please see my comment below.
the warning tends to be emitted multiple times when using autosummary / autodoc. Is that something I can fix?
I don't have an answer. I need to investigate what happened in this case. Can I reproduce that in this test case? If true, I'll take a look.
regarding continuation lines: I adopted the recommendation in numpydoc (it seems that we'd need to modify the rst standard on definition blocks if we wanted to do something else). Any objections?
Of course, yes. no problem. The NumpyDocstring
class has followed to numpydoc format.
when extending a library using inheritance, the docstrings of inherited methods often contain references to other objects within the same namespace (without specifying the qualname). Is there a existing way to specify packages that should be checked if it can't be found in the current namespace?
This is another problem. In my memory is correct, a similar issue was already filed (I can't find it now...).
I just noticed default doesn't have a standardized format. pandas seems to mostly use default but with the current implementation that is really difficult to support (but not impossible). Should I try to get this to work?
I don't know about pandas at all. So I can't determine that.
In addition, the goal of napoleon (I supposed) is not supporting all of the formats for docstrings. They are only for google's and numpy's docstring. So no reason to support it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, :class: role is used. As an exception, :obj: is used for None value.
That still doesn't get a consistent link style: the docstring of the test project's test.multiply
results in
:param a: first factor
:type a: :term:`array-like <array_like>`
:param b: second factor
:param \*variables: parameters without use
:type \*variables: str
:param mod: optionally compute the modulo of the product
:type mod: :term:`array-like <array_like>`, *optional*
:param files: save each value of the results into a new file
:type files: :class:`list` of :class:`str` or :class:`list` of :class:`os.PathLike` or :term:`dict-like <mapping>`
:param save_format: file format
:type save_format: ``{"ma{icious", "options", "that need a line break"}``, *default*: ``"options"``
:param test: optional parameter that does absolutely nothing
:type test: *optional*
:param \*\*variable_kwargs: kwargs form of variables
but the rendered version (logs) displays certain objects as preformatted link. Is there any way to make that a plain link, just like the single str
for *variables
? Or should I just leave this as-is?
I don't have an answer. I need to investigate what happened in this case. Can I reproduce that in this test case? If true, I'll take a look.
Unfortunately not by the test case, but with the project linked above. I guess that's because I'm using both autosummary
and autodoc
?
Edit: I can't reproduce it anymore. Not sure what happened, but I guess it's fixed?
Unfortunately there no good way. The autodoc-process-docstring that napoleon uses does not send a location of the docstring. It might be able to use self._obj to obtain the name of the docstring owner.
Thanks, that works; now only the line number is missing. Since all other warnings about docstrings also seem to be missing the line number I guess that's not so bad.
This is another problem. In my memory is correct, a similar issue was already filed (I can't find it now...).
I'll try to find that issue.
Edit: I can't find it either. However, I can work around that issue using the new napoleon_type_aliases
setting, so this shouldn't be a blocker.
I don't know about pandas at all. So I can't determine that.
In addition, the goal of napoleon (I supposed) is not supporting all of the formats for docstrings. They are only for google's and numpy's docstring. So no reason to support it.
pandas
uses numpydoc
, but extends it slightly to allow documenting default values as part of the type. I'm also proposing to extend numpydoc
, with a slightly different syntax (default: None
instead of pandas
' default None
). Is it okay to include this for now (and ask on the numpydoc
issue tracker) or should I wait for a response before including that?
with this I think there's only a few issues left:
|
Sorry, my explanation above was not correct. It uses And I can't find the way to build type descriptions having the same style.
I found issues related to this: #5977, #4961 and #272.
At present, the napoleon extension officially supports only google style and numpy style docstrings. So it should not add a support non-official notation. I'll never accept to support non-numpydoc syntax as numpydoc. The only I can agree is making napoleon extendable from 3rd party's extension.
Oh, sorry. 3.x branch is better to merge. About |
Hmm... well, the links work so while the current state is not ideal, this can be fixed in a new PR.
So that means if I can get this to become official Edit: see numpy/numpydoc#284
I'll change that Edit: done
I changed the code so the type spec only gets parsed with About the warnings: those are the ones that are generated for wrong type specs (invalid numpydoc), e.g. if value sets are not closed, end quotes are missing or there's a missing space after a comma. Those warnings have been nitpicky before (because linking to a value set failed) so if you tell me how I can change these to |
I added support for the other style, too. With that, I guess the only thing left is to decide what to do with the warnings? |
Thank you for your update. I can accept your patch now (with nits). On the other hand, it would be wonderful if we'll support numpy types even for
I think it's acceptable warnings if the mark-ups are really broken. So it is not needed to change them nitpicky.
I'm not familiar with numpydoc. So I'll merged this as is. From the point of view of a maintainer, it would be better to describe (by comment, docstring, or something) why it is implemented if it is not a part of "official" spec. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why it is implemented if it is not a part of "official" spec
I think that's because they didn't update the docstring guide in quite a while. I'll try submitting a PR changing the docstring guide (the official spec).
it would be wonderful if we'll support numpy types even for
napoleon_use_param = False
.
Sure, but I guess that would change the current behavior of not linking at all.
done. I also changed the link to the official spec to its new location (the old one is just a in-text redirect). |
I think it should be documented to upstream's doc if they have special meaning for the colon after "default" keyword. So it sounds nice :-) |
the colon doesn't have a special meaning, it's completely optional. |
:param arg1: Description of `arg1` | ||
:type arg1: mypackage.CustomType | ||
:param arg2: Description of `arg2` | ||
:type arg2: :term:`dict-like <mapping>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. versionadded:: 3.2
tag is needed at the end of description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update this after merging.
Just Merged. Thank you for your great work! |
Thanks for this--much more sophisticated and complete than my hackish workaround. |
Subject:
napoleon
should translate the type fieldsFeature or Bugfix
could be both?
Purpose
As reported in #6861,
napoleon
simply puts everything after the colon of anumpy
-style parameter description (e.g. theint
inval : int
) into a:type:
field. This works most of the time, butnumpydoc
also uses certain keywords (optional
,default
), value sets (e.g.{"val1", "val2"}
) and literals (as indefault: "val2"
), among others. Right now, using those will raise nit-picky warnings and possibly also result in incorrect parameter documentation.This PR adds a new role (named
noref
, but could also beliteral
ortext
; I'm not confident in my ability to come up with good names) that is then used to make the type field ignore the keywords / literals above.The type preprocessing also allows adding a mapping of terms to other links (see the
translations
dict), so terms likedict-like
orarray-like
/array_like
can be mapped tomapping
using a configuration option. It isn't restricted to terms, though, any kind of role should be possible.I don't really know if this is the best approach to the problem described in #6861 (in that issue, the suggestion to create a new section for optional parameters came up) but I'm submitting this to start making progress.
Relates