Skip to content

Proposal: add an optional copy argument to np.reshape #9818

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

Open
shoyer opened this issue Oct 3, 2017 · 9 comments
Open

Proposal: add an optional copy argument to np.reshape #9818

shoyer opened this issue Oct 3, 2017 · 9 comments

Comments

@shoyer
Copy link
Member

shoyer commented Oct 3, 2017

Inspired by my discussion with @charris in #9810, I would like to propose adding an optional copy argument to np.reshape() and ndarray.reshape().

Proposed semantics:

  • copy=None: copy only if necessary (default).
  • copy=True: always copy.
  • copy=False: never copy. Raise an error if the data cannot be reshaped without a copy.

This would allow users to ensure desired behavior for reshape and alleviate the primary use case for modifying ndarray.shape.

@seberg
Copy link
Member

seberg commented Oct 3, 2017

Always agreed with that, the only slight annoyance is that we need yet another C-API version probably.

@eric-wieser
Copy link
Member

Yep, this seems like a very good idea to me too.

@mhvk
Copy link
Contributor

mhvk commented Oct 4, 2017

Agreed too, the only reason I use assignments to shape is because in some cases I want to be absolutely sure there won't be a copy for a very large array. I can similarly see the use of being able to ensure a copy is in fact made.

@seberg
Copy link
Member

seberg commented Oct 4, 2017

@mhvk and the fun thing is, it does not really help you (except in the sense of an assert statement), because the shape assignment actually does the copy, then sees its a copy and raises an error.

EDIT: Just to note, I am not sure if we have to Deprecate it, but we still need to discourage it in the docs, its a bad habit, but dunno if there might be a handful of actual use cases.

@charris
Copy link
Member

charris commented Oct 4, 2017

ISTM that reshape can also replace ravel.

@charris
Copy link
Member

charris commented Oct 4, 2017

Note that we will need to make sure that this doesn't break current subclasses of ndarray that do not implement the new keyword.

@seberg
Copy link
Member

seberg commented Oct 4, 2017

True, about ravel, there is a problem with ravel, in that it makes sure that the output array is contiguous, which was a regression we once had and because someone (maybe scipy/some scikit) relied on it I think.

@shoyer
Copy link
Member Author

shoyer commented Oct 6, 2017

On the C-API side, I suppose it would make sense to use enums here rather PyObject for the extra argument? e.g., NPY_COPY_DEFAULT, NPY_COPY_TRUE, NPY_COPY_FALSE?

@rgommers
Copy link
Member

In gh-11884, @seberg proposed the same thing as this issue. With the extra motivation that users now modify arr.shape instead of using reshape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants