-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
ENH: Add the axis
and ndim
attributes to np.AxisError
#19459
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
:toctree: generated/ | ||
|
||
AxisError |
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.
A few other candidates the might be worthwhile to add here:
Lines 3646 to 3653 in 89babbd
# Warnings | |
class ModuleDeprecationWarning(DeprecationWarning): ... | |
class VisibleDeprecationWarning(UserWarning): ... | |
class ComplexWarning(RuntimeWarning): ... | |
class RankWarning(UserWarning): ... | |
# Errors | |
class TooHardError(RuntimeError): ... |
Co-Authored-By: Eric Wieser <425260+eric-wieser@users.noreply.github.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
Currently I'm in favor of either using the current approach with |
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.
LGTM, I am happy with the patterns as is. Delaying string construction might be worthwhile, but doesn't seem too important?
The one thing I thought might be nice, is to note that AxisError
subclasses both IndexError
and ValueError
, since that is important for try/except
clauses. But also fine to improve later, it isn't like we had docs before...
…lass Co-Authored-By: Sebastian Berg <sebastian@sipsolutions.net>
It seems like an easy (and sensible) change to make though, so done as of 16964ae. |
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.
To be clear, the alternative I'm suggesting is:
class AxisError(ValueError, IndexError):
def __init__(self, axis, ndim=None, msg_prefix=None):
if ndim is None and msg_prefix is None:
# single-argument form (for backwards compatibility) sets just the message
self._msg = axis
self.axis = None
self.ndim = None
else:
self._msg = msg_prefix
self.axis = axis
self.ndim = ndim
def __str__(self):
if self.axis is not None and self.ndim is not None:
msg = "axis {} is out of bounds for array of dimension {}".format(
self.axis, self.ndim)
if self._msg is not None:
msg = "{}: {}".format(self._msg, msg)
return msg
else:
return self._msg
Which also provides an AxisError(10, 3)
-style __repr__
for free
Co-Authored-By: Eric Wieser <425260+eric-wieser@users.noreply.github.com>
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.
Thanks for the iteration. The new docs are great!
I've suggested a possible longer explanation of why we subclass indexerror and valueerror - feel free to reword it. The versionadded
definitely seems like a good thing to mention, but I'm not sure how far up the docstring it belongs.
Co-Authored-By: Eric Wieser <425260+eric-wieser@users.noreply.github.com>
Great also following up with the |
This PR adds the new
axis
andndim
attributes to thenp.AxisError
class, an addition inspired by similarchanges introduced to
AttributeError
in Python 3.10.It also provided an excuse to update the classes' documentation and tests, both of which were previously rather lacking.
Examples