8000 MAINT: Speed up normalize_axis_tuple by about 30% by mhvk · Pull Request #11518 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

MAINT: Speed up normalize_axis_tuple by about 30% #11518

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 9, 2018
Merged

MAINT: Speed up normalize_axis_tuple by about 30% #11518

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c42cc3
MAINT: Speed up normalize_axis_tuple by about 30%
mhvk Jul 6, 2018
4bed228
MAINT: Restore previous behavior on subclasses
eric-wieser Jul 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions numpy/core/numeric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1509,11 +1509,14 @@ def normalize_axis_tuple(axis, ndim, argname=None, allow_duplicate=False):
--------
normalize_axis_index : normalizing a single scalar axis
"""
try:
axis = [operator.index(axis)]
except TypeError:
axis = tuple(axis)
axis = tuple(normalize_axis_index(ax, ndim, argname) for ax in axis)
# Speed-up most common cases.
if not isinstance(axis, (list, tuple)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be more correct to check type(axis) in (list, tuple), since subclasses might override __index__.

I was tempted to change it to just type(axis) is tuple, to encourage people to use tuple rather than list axis arguments....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if someone subclasses list or tuple, one should reasonably expect that the object can be iterated over, not also treated as an integer! (I'd actually prefer to just test for something being iterable, but things like np.iterable are way slower than the isinstance check.)

As for tuple - I can at least change the order!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable or not, I'd prefer that we don't introduce an unnecessary behavior change now.

type(axis) in (tuple, list) sounds like a good approach to me

try:
axis = [operator.index(axis)]
except TypeError:
pass
# Going via an iterator directly is slower than via list comprehension.
axis = tuple([normalize_axis_index(ax, ndim, argname) for ax in axis])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this is the case...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think list comprehension is special-cased, not constructing an iterator but an actual for-loop with known size.

if not allow_duplicate and len(set(axis)) != len(axis):
if argname:
raise ValueError('repeated axis in `{}` argument'.format(argname))
Expand Down
0