-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
MAINT: ndarray.__repr__
should not rely on __array_function__
#12212
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
Conversation
``ndarray.__repr__`` and ``ndarray.__str__`` should not rely upon ``__array_function__`` internally, so they are still well defined on subclasses even if ``array_repr`` and ``array_str`` are not implemented. Fixes numpygh-12162
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unit test is certainly sensible -- I tried this locally a few days ago.
Probably out of scope, but I wonder what happens if you set a custom array_repr()
that simply points to the one used by ndarray
, but then write your own array_add
. Would the repr padding calculations (which sometimes use the array add) then go through the custom array_add
instead of the ndarray
version?
We don't expose the non-overloaded version of |
I like that this splits up the numpy array_repr api and the array_repr implementation. I mentioned in another thread that ducktype implementors may want to reuse the implementation (after some changes to array2string), but before now there was no way to call it separately from the api method. I still have a feeling this isn't the last change we will make here. For instance, I'm not 100% convinced that Despite all that, this PR seems like a step forward for now, to get things in a working state, so I would merge it. (Needs rebase though). |
"One-off helper functions" seems cleaner to me, but already expose them in NumPy's top level API which suggests they need If I were starting from scratch I would probably only expose |
All right, LGTM, merging. Thanks Stephan. |
ndarray.__repr__
should not rely on __array_function__
ndarray.__repr__
andndarray.__str__
should not rely upon__array_function__
internally, so they are still well defined on subclasses even ifarray_repr
andarray_str
are not implemented.Fixes gh-12162