Sitelet https://web.archive.org/web/20221111081133/https://github.com/numpy/numpy/pull/21853
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: Update the docstring for np.around #21853

Merged
merged 3 commits into from Oct 19, 2022

Conversation

InessaPawson
Copy link
Member

@InessaPawson InessaPawson commented Jun 26, 2022

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
Contributor

@rossbar rossbar left a comment

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!

@@ -3243,12 +3243,14 @@ def around(a, decimals=0, out=None):
See Also
--------
ndarray.round : equivalent method

numpy.round_ : equivalent method
Copy link
Contributor

@rossbar rossbar Jun 26, 2022

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
Member

@rkern rkern Jun 26, 2022

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.).

ceil, fix, floor, rint, trunc


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

@rossbar rossbar Jun 26, 2022

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
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
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
41 checks passed
kernel-loophole added a commit to kernel-loophole/numpy that referenced this pull request Oct 19, 2022
DOC: Add note to clarify relationship between round and around (numpy#21853)
@InessaPawson
Copy link
Member Author

InessaPawson commented Oct 31, 2022

@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
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.

None yet

4 participants