10000 np.swapaxes doesn't always return a view · Issue #5260 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

np.swapaxes doesn't always return a view #5260

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

Jump to bottom
Closed
jaimefrio opened this issue Nov 4, 2014 · 7 comments
Closed

np.swapaxes doesn't always return a view #5260

jaimefrio opened this issue Nov 4, 2014 · 7 comments

Comments

@jaimefrio
Copy link
Member

Related somehow to the discussion in #5258. The docs for np.swapaxes state that it returns a view if the first argument is an ndarray, but it has a fast path for axis1 == axis2, where it actually returns the original array incref'ed:

>>> a = np.arange(60).reshape(3, 4, 5).copy()
>>> a.base is None
True
>>> a.swapaxes(0, 1).base is None
False
>>> a.swapaxes(0, 0).base is None
True
>>> a.swapaxes(20, 20).base is None  # that this works is bad!!!
True
>>> a.transpose(range(3)).base is None
False

I prefer that it always returned a view, even if it is of the whole array unchanged, as the docs say, and as e.g. np.transpose does, even if the array is unchanged, and will be more than happy to submit a PR. If there is any good reason why this should not be the case, then at least the docs should be updated, and the check for axis1 == axis2 be moved to after they have both been checked for in-boundness.
@njsmith
Copy link
Member
njsmith commented Nov 4, 2014

I agree that the lack of bounds checking is a bug. I'm not sure it matters
whether we return a view or the original array -- I guess it affects things
like .resize? -- but if you want to fix it then I can't see any problem
with that either.

On Tue, Nov 4, 2014 at 5:44 PM, Jaime notifications@github.com wrote:

Related somehow to the discussion in #5258
#5258. The docs for np.swapaxes
state that it returns a view if the first argument is an ndarray, but it
has a fast path for axis1 == axis2, where it actually returns the
original array incref'ed:

a = np.arange(60).reshape(3, 4, 5).copy()
a.base is None
True
a.swapaxes(0, 1).base is None
False
a.swapaxes(0, 0).base is None
True
a.swapaxes(20, 20).base is None # that this works is bad!!!
True
a.transpose(range(3)).base is None
False

I prefer that it always returned a view, even if it is of the whole array
unchanged, as the docs say, and as e.g. np.transpose does, even if the
array is unchanged, and will be more than happy to submit a PR. If there is
any good reason why this should not be the case, then at least the docs
should be updated, and the check for axis1 == axis2 be moved to after
they have both been checked for in-boundness.

—
Reply to this email directly or view it on GitHub
#5260.

Nathaniel J. Smith
Postdoctoral researcher - Informatics - University of Edinburgh
http://vorpus.org

@seberg
Copy link
Member
seberg commented Nov 4, 2014

Sounds reasonable. I guess ideally returning the array or returning a view does not really matter, because ndarray is almost immutable (the container part). Resize and arr.shape = (1, 2, 3)/arr.strides = (1, 2) and even arr.dtype are notable exception. Except shape, none of these are probably used, but then view creation should be fast enough...

@jaimefrio
Copy link
Member Author

There is actually the exact same issue with rollaxis, see here. I had some old code that tripped on this, but I can't fully recall what the exact issue was.

@charris
Copy link
Member
charris commented Nov 4, 2014

Always returning a view sounds right to me on first thought.

@charris
Copy link
Member
charris commented Nov 11, 2014

Is there agreement on always returning a view?

@charris charris mentioned this issue Nov 11, 2014
Closed
@jaimefrio
Copy link
Member Author

My work schedule has been crazy this past few weeks, but I will make a pull request later tonight. Will also ping the list when it is ready.

@njsmith
Copy link
Member
njsmith commented Nov 11, 2014

Everyone seems lukewarmly in favor :-)

@charris charris mentioned this issue Jan 19, 2015
Merged
charris added a commit that referenced this issue Jan 21, 2015
@charris
Merge pull request #5468 from jaimefrio/swapaxes_view
960433e 
ENH: Make swapaxes always return a view. Fixes #5260
@jaimefrio jaimefrio mentioned this issue Mar 19, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@seberg @charris @njsmith @jaimefrio
0