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

cube.transpose is an in-place operation #2615

Closed
pp-mo opened this issue Jun 21, 2017 · 7 comments
Closed

cube.transpose is an in-place operation #2615

pp-mo opened this issue Jun 21, 2017 · 7 comments
Labels
Experience: High Stale A stale issue/pull-request

Comments

@pp-mo
Copy link
Member

pp-mo commented Jun 21, 2017

I think it's pretty nasty and unexpected to have cube.transpose() modify in-place
That is, especially, as it seems to mimic numpy "transpose", which is not in-place.
Plus, we just don't have many in-place operations, largely by design (I think).

Thus in #2604 ...

  • I had a test cube and a sample result array which should match a derived coordinate.
  • I applied transpose operations to both, but the cube behaves differently to the result, when it should have been the same.

I think we should review this as it's peculiar and unexpected, and breaks the general simlilarity with numpy.
Arguably the numpy function could more clearly be named "transposed" (a la Python "sorted").
Obviously, that ship has sailed, but we could use that naming for a cube method.

Propose:

  • provide copying version called "cube.transposed()" instead
  • deprecating the existing "cube.transpose" -- or simply remove it from Iris 2.0 ??
@pp-mo pp-mo added this to the v2.0 milestone Jun 21, 2017
@cpelley
Copy link

cpelley commented Jun 22, 2017

+1 for cube.transpose returning a cube, however it is important to us that it does not consume any more memory for the data or coordinates i.e. the cube returned should be a view of the original cube. This would also be in keeping with ndarray.transpose behaviour. Not sure where transposed comes into this as numpy uses transpose.

@pelson
Copy link
Member

pelson commented Oct 19, 2017

Taking this out of the v2.0.0 milestone - we aren't going to hit this in time, but can very readily add this as a future flag during the v2.x series. Of course, we will need to figure out data sharing before we can do this though (again, not going to happen for v2.0.0).

@github-actions
Copy link
Contributor

In order to maintain a backlog of relevant issues, we automatically label them as stale after 500 days of inactivity.

If this issue is still important to you, then please comment on this issue and the stale label will be removed.

Otherwise this issue will be automatically closed in 28 days time.

@github-actions github-actions bot added the Stale A stale issue/pull-request label Aug 10, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2021

This stale issue has been automatically closed due to a lack of community activity.

If you still care about this issue, then please either:

  • Re-open this issue, if you have sufficient permissions, or
  • Add a comment pinging @SciTools/iris-devs who will re-open on your behalf.

@github-actions github-actions bot closed this as completed Sep 8, 2021
@monishanatchiar
Copy link

I agree that in-place operations need to be reduced, or we should at least have an option like in_place=False. Currently, I have to create a copy of the iris.cube.Cube to not transpose it in-place.

@cpelley
Copy link

cpelley commented Dec 15, 2023

I appreciate numpy is not consistent itself here but 'typically' method calls are in-place and calling an operation via function call would return a copy (numpy.ndarray.transpose method call actually returns a view... rather than internally transposing a view on the object itself i.e. ~in-place).

i.e. anyway, so for iris what about:

return copy

iris.transpose(cube, ...)

in-place transpose of cube

cube.transpose(...)

...we should at least have an option like in_place=False...

Numpy makes use of the optional out=None parameter for this purpose and is more flexible:
e.g., see cumsum

I think a contract is required on the UI. Ideally consistency, predictability AND flexibility.

Cheers

@trexfeathers
Copy link
Contributor

Xref #5007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience: High Stale A stale issue/pull-request
Projects
None yet
Development

No branches or pull requests

5 participants