DOC: Update the docstrings for np.around and np.round_#22527
DOC: Update the docstrings for np.around and np.round_#22527rgommers merged 4 commits intonumpy:mainfrom
Conversation
numpy/core/fromnumeric.py
Outdated
| Notes | ||
| ----- | ||
| `~numpy.round` is often used as an alias for `~numpy.around`. | ||
| `~numpy.round` and `~np.round_` are disrecommended |
There was a problem hiding this comment.
I confess this is a confusing subject :) There is a round defined in numpy/array_api/_elementwise_functions.py, apparently it will be part of the common array_api. @asmeurer Is that the case? And should we then recommend it as portable? I think that is the main concern here.
Note that there is also an ndarray.round method, and round is used a lot in the Notes section of around. For those interested, round itself is defined in numpy/ma/core.py and numpy/core/__init__.py
There was a problem hiding this comment.
The functions in array_api shouldn't be recommended as they only work on array_api arrays. But we should recommend the functions that are part of the spec, such as round, as other libraries will be using those.
There was a problem hiding this comment.
Maybe we should mention spec functions in the See Also section? This needs some discussion. @rgommers Thoughts?
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Documenting array_api in general is an open question. It also relates to the still somewhat open question of whether numpy.array_api even belongs in NumPy at all (it's still tagged as experimental). I think it's better to discuss it on the mailing list, or one of the dev calls (if you ever want me to join one of the dev calls just let me know beforehand).
There was a problem hiding this comment.
@Inessa I think Ross' suggestion below is a good one. Short version: folks should not use np.round_, but np.round is fine. At some point we may want to prefer np.round, but that should wait on a discussion as to how we want to handle functions common to numpy and the array_api.
There was a problem hiding this comment.
I agree with the outcome here, and with what @rossbar and @asmeurer say.
Maybe we should mention spec functions in the See Also section? This needs some discussion. @rgommers Thoughts?
I'd say no, we should just aim for those names to be the defaults. No need to reference or cross-link to the standard API docs in individual docstrings. Aaron already wrote a separate narrative docs section which details all the differences. Let's just shrink that over time.
rossbar
left a comment
There was a problem hiding this comment.
IMO I don't think these recommendations should be added to the docstring. AFAIK the motivation behind round_ was to prevent masking of the Python built-in round in the days where pylab or patterns like from numpy import * were more prevalent. My personal vote would be to deprecate round_, which means we would end up removing all references to it in docstrings anyways.
Re: around - NumPy doesn't recommend that users use the a-versions of the other functions, e.g. np.amin over min, so I'm not sure why that would change here. IMO the situation with around is even worse as the function name is ambiguous: it's supposed to stand for "array round" but the function name around can easily be mistaken for the word "around" as in "give me the values around this other value".
For this PR, the thing that makes the most sense to me is to remove round_ from the autosummary. around is already in the See Also section of the round docstring, which IMO is sufficient to get users pointed to correct docs.
|
By the way, why doesn't |
The array API spec only does this convention for |
|
array rounding is not quite compatible with returning Python integers when the digit is None. Might be that is the reason why nobody ever really jumped on implementing it. |
|
>>> class Test:
... def __round__(self, n=None):
... return self
>>> round(Test())
<__main__.Test object at 0x113e70ac0>
>>> round(Test(), 1)
<__main__.Test object at 0x12539d9d0> |
|
Yes, but it is documented to and Python does so. Of course you are right, we really could not follow. For arrays that is not surprising, for scalars it might be a bit (but maybe also less so when |
|
Looks like there's already an issue for |
|
This bit still needs fixing: |
|
@charris I updated the info about the usage of |
|
IIUC we can close this now that #22615 is in. Adding more references to |
@rossbar I’d imagine it would take a while to deprecate |
`round_` was removed from the docs so should not be referenced, and the separate Notes section isn't needed. [skip azp] [skip actions] [skip cirrus]
55e31b0 to
efbedf1
Compare
rgommers
left a comment
There was a problem hiding this comment.
This doesn't actually fix gh-19717 I think (the point of that was to have numpy.round as a separate doc entry, so it shows up in doc tables and is discoverable etc.). However, I remembered this existed and want to make an update to round_, so I thought I'd push a change and get this in. It's a clarification to the round_ docstring that can't hurt.
|
Merged, thanks @InessaPawson & reviewers. |
Update the docstrings for np.around and np.round_ to address #19717. Added additional info based on the comments in #21853.