DEP: Schedule unused C-API functions for removal/disabling #15427
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5289f1c to
8596d49
Compare
mattip
reviewed
Jan 26, 2020
mattip
reviewed
Jan 26, 2020
numpy/core/src/umath/ufunc_object.c
Outdated
Member
Author
There was a problem hiding this comment.
Would have liked to claim it was on purpose to proof that really nobody uses it ;). Waiting for mailing list replies, but my guess is, nobody will really care (at least if I read your reply as "OK with some reservations", Chuck).
8596d49 to
52cb0cb
Compare
eric-wieser
reviewed
Jan 27, 2020
mattip
reviewed
Jan 29, 2020
2936845 to
deb8470
Compare
The function has no known usage outside of NumPy. It is fairly complex to use due to how it behaves with scalars, and, at least with a given "flexible" dtype would require additional access to the private PyArray_AdaptFlexibleDType function. Removing the function from public API will simplify refactor work of the dtype and dimension recovery.
This function provides mainly the same API as PyObject_Call() with the exception of skipping ufunc overrides and output array wraps. It is fairly unwieldy to use, since ownership of all inputs and outputs is transferred, but otherwise it still uses args and kwargs as inputs. As such, no known usage exists and PyObject_Call seems a reasonable replacement for possible usage. Keeping it around means that we would may have to add tests or risk breaking this function when ufunc code is refactored.
This function is currently undocumented. It is not used in NumPy and seems useless outside of NumPy. This is due to the fact that user datatypes are not actually capable of leveraging this function! They are stored in a different way, so that their loops cannot be identified by an index.
deb8470 to
468ccdd
Compare
Member
|
Do you still want to add tests for these? I guess we don't need a release note since the claim is that the functions are unused. |
Member
Author
|
I am not particularly in love with adding tests, all they would do is see the warning is correct. But I am happy to do this if anyone prefers. Forgot about release notes, can't hurt, added them. |
mattip
reviewed
Feb 1, 2020
Member
There was a problem hiding this comment.
unused and have been deprecated.
If you can say what the other API functions are in two words that would be nice
These functions are believed to be unused, or at least largely unused.
b98414e to
5ee5012
Compare
Member
|
Thanks @seberg |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This deprecates three functions which I believe are unlikely to have any use case outside NumPy. My current hope is that we can remove these (and plausibly more) immediately after a 1.19 release and manage to put in a large hunk of dtype changes than.
The last function was not even documented, and is pretty much useless, unless possibly for someone writing
datetimeufuncs outside of NumPy.I probably need to add tests for this ;), and will ping the mailing list. I will admit that none of the functions are completely unreasonable to expose, but their implementation details seem to expose unnecessary complexity at this time.