Skip to content

DOC: Update the docstring for np.around#21853

Merged
rossbar merged 3 commits intonumpy:mainfrom
InessaPawson:nparound-update
Oct 19, 2022
Merged

DOC: Update the docstring for np.around#21853
rossbar merged 3 commits intonumpy:mainfrom
InessaPawson:nparound-update

Conversation

@InessaPawson
Copy link
Copy Markdown
Member

Updating the docstring for np.around to address #19717.

Co-authored-by: Mukulika Pahari mukulikapahari@gmail.com, Aparna Bushan aparna.bushan@auto-grid.com

Co-authored-by: Mukulika Pahari <mukulikapahari@gmail.com>, Aparna Bushan <aparna.bushan@auto-grid.com>
Copy link
Copy Markdown
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

This generally seems fine to me (see inline comments) but I don't think it's sufficient to close #19717.

If I understand the issue correctly, the main problem is that users can't find numpy.round in the generated html docs. The fix for this problem would be to add round to the appropriate autosummary so that the stubs are generated correctly; i.e. add "round" to the autosummary in routines.math.rst.

I'd also argue in favor of removing round_ from the list, but that is its own discussion which should be left to a separate PR!

--------
ndarray.round : equivalent method

numpy.round_ : equivalent method
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should be linking to round_ - users should probably prefer numpy.round, which is literally the same function with a different (less clunky) name.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both np.round and np.round_ are backwards-compatibility aliases. Neither should be used, and when they are listed as aliases, we should probably note that whenever they are mentioned. They have equivalent status and should probably both be listed here as disrecommended backwards-compatibility aliases.

IIRC, we originally had numpy.round, but that shadowed the builtin round() when from numpy import * was used (which was common at the time). For backwards compatibility, we renamed it to np.round_ and left np.round as an alias but removed it from the np.__all__ list. Sometime later, we decided that the underscore-appending naming scheme was too clunky for the official preferred versions of such functions and settled on the a prefix (around, amin, amax, etc.).


Notes
-----
np.round is often used as an alias for np.around.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Using single backticks will make these terms link to their definitions.

Suggested change
np.round is often used as an alias for np.around.
`round` is often used as an alias for `around`.

If you want to be very specific about the namespaces (which may make sense in this case) you can instead use the fully-qualified name, e.g. `numpy.round`.

If you really want to use the np alias (and have the terms not be links), then these should be wrapped in double-backticks so that they are formatted as code, e.g. ``np.round``

@charris
Copy link
Copy Markdown
Member

charris commented Sep 29, 2022

We should finish this up. @rossbar What remains to be done. I also wonder if we should suggest that np.around should be preferred to the aliases np.round and np.round_.

@rossbar
Copy link
Copy Markdown
Contributor

rossbar commented Oct 19, 2022

I took the liberty of adding a few final touchups, but LGTM - thanks @InessaPawson & @Mukulikaa !

@rossbar rossbar merged commit 7776bc6 into numpy:main Oct 19, 2022
@InessaPawson
Copy link
Copy Markdown
Member Author

@rossbar, thank you for taking care of it! Should I open a discussion about the deprecation of round_ on the mailing list?

@InessaPawson
Copy link
Copy Markdown
Member Author

InessaPawson commented Oct 31, 2022

@rossbar Do we still want to remove round_ from the autosummary in https://github.com/numpy/numpy/blob/main/doc/source/reference/routines.math.rst#rounding?

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