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

DOC: Include the versionadded to the isnat documentation. #11630

Merged
merged 1 commit into from
Jul 29, 2018
Merged

DOC: Include the versionadded to the isnat documentation. #11630

merged 1 commit into from
Jul 29, 2018

Conversation

MSeifert04
Copy link
Contributor

@MSeifert04 MSeifert04 commented Jul 27, 2018

Just for reference: It's mentioned as new in the 1.13 release notes

New np.isnat ufunc tests for NaT special values.

@@ -1740,6 +1740,8 @@ def add_newdoc(place, name, doc):
"""
Test element-wise for NaT (not a time) and return result as a boolean array.

.. versionadded:: 1.13.0

Copy link
Member

Choose a reason for hiding this comment

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

Should be preceded by a Notes heading, see other examples in the file (although in _adddocs.py I do see examples that do not use the heading).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the more "recent" (1.12 and 1.13) .. versionadded were included in the general description. Also it seemed a bit wasteful to add another section just for the versionadded. But if you like it better in the Notes I can change that.

Copy link
Member

Choose a reason for hiding this comment

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

We might want to revisit that. I think it makes more sense to put the versionadded in the extended summary where it is easily noticed, likewise, when arguments are added we put the versionadded with the argument documentation. @rgommers Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Difference is, the arguments apply to just that argument, whereas this applies to the entire function. I think this is fine as is.

Copy link
Member

Choose a reason for hiding this comment

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

@eric-wieser Yep, I think that is fine and prefer it that way. However, the documentation for that says to put it in the Notes, which I haven't done myself (tsk, tsk), so we should fix the documentation.

@mattip mattip merged commit 0df3563 into numpy:master Jul 29, 2018
@mattip
Copy link
Member

mattip commented Jul 29, 2018

Thanks @MSeifert04

@MSeifert04 MSeifert04 deleted the add_versionadded_to_isnat_docs branch July 29, 2018 19:59
@charris charris changed the title [DOC]: Include the versionadded to the isnat documentation. DOC: Include the versionadded to the isnat documentation. Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants