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

ENH: Add the axis and ndim attributes to np.AxisError #19459

Merged
merged 15 commits into from Jul 20, 2021

Conversation

BvB93
Copy link
Member

@BvB93 BvB93 commented Jul 12, 2021

This PR adds the new axis and ndim attributes to the np.AxisError class, an addition inspired by similar
changes 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

In [1]: import numpy as np

In [2]: exc = np.AxisError(2, 1)
   ...: print(exc)
axis 2 is out of bounds for array of dimension 1

In [3]: exc.axis
Out[3]: 2

In [4]: exc.ndim
Out[4]: 1

:toctree: generated/

AxisError
Copy link
Member Author

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:

numpy/numpy/__init__.pyi

Lines 3646 to 3653 in 89babbd

# Warnings
class ModuleDeprecationWarning(DeprecationWarning): ...
class VisibleDeprecationWarning(UserWarning): ...
class ComplexWarning(RuntimeWarning): ...
class RankWarning(UserWarning): ...
# Errors
class TooHardError(RuntimeError): ...

BvB93 and others added 2 commits July 12, 2021 23:28
Co-Authored-By: Eric Wieser <425260+eric-wieser@users.noreply.github.com>
Co-authored-by: Eric Wieser <wieser.eric@gmail.com>
@BvB93
Copy link
Member Author

BvB93 commented Jul 15, 2021

Currently I'm in favor of either using the current approach with cls._reconstructor or
to use the updated__reduce__ from #19459 (comment). Is there any preference?

Copy link
Member

@seberg seberg left a 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>
@BvB93
Copy link
Member Author

BvB93 commented Jul 19, 2021

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

It seems like an easy (and sensible) change to make though, so done as of 16964ae.

Copy link
Member

@eric-wieser eric-wieser left a 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>
@seberg seberg added the 62 - Python API Changes or additions to the Python API. Mailing list should usually be notified. label Jul 19, 2021
Copy link
Member

@eric-wieser eric-wieser left a 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.

Bas van Beek and others added 2 commits July 20, 2021 11:03
Co-Authored-By: Eric Wieser <425260+eric-wieser@users.noreply.github.com>
@seberg
Copy link
Member

seberg commented Jul 20, 2021

Great also following up with the __str__ method :). Lets put this in, thanks Bas and Eric.

@seberg seberg merged commit f435332 into numpy:main Jul 20, 2021
@BvB93 BvB93 deleted the TooHardError branch July 20, 2021 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
01 - Enhancement 62 - Python API Changes or additions to the Python API. Mailing list should usually be notified.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants