Skip to content

Conversation

@cpelley
Copy link

@cpelley cpelley commented Jul 21, 2015

Addition to iris.util for convenient inversion of a specified coordinate on a cube or list of cubes.

@cpelley
Copy link
Author

cpelley commented Jul 21, 2015

Test failures not relating to the changes of this PR

@rhattersley
Copy link
Member

Test failures not relating to the changes of this PR

See #1730.

Copy link
Member

Choose a reason for hiding this comment

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

There's already a lat_lon_cube function in this module which does a very similar (albeit more limited) job. It'd be nicer if we could just extend lat_lon_cube.

@ajdawson
Copy link
Member

If I reverse a dimension that also has a 1D AuxCoord describing it, will that be reversed too? And what about if there is a 2D AuxCoord overlapping the dimension, will it be reversed along the required axis?

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't really do enough to warrant existing as a separate function.

@rhattersley
Copy link
Member

There an iris.util... function for this ... demote_dim_coord_to_aux_coord. More on this topic later...

Perhaps this is a good time to consider whether iris.util is really the best place for this function (and a bunch of other functions that are already in there!). The documentation shows the module is a mess already - a real mix of arbitrary helpers (timers, create_temp_filename, ...), numeric helpers (monotonic, reverse, ...), Cube operations (as_compatible_shape, describe_diff, ...). Surely we can do better...?

(NB. This is somewhat related to @bjlittle's upcoming "add-ons"/"extensions"/"incubator" repo.)

@cpelley
Copy link
Author

cpelley commented Jul 21, 2015

Thank you for your interest in this @rhattersley

..the module is a mess already - a real mix of arbitrary helpers..

I very much agree.

This is somewhat related to @bjlittle's upcoming "add-ons"/"extensions"/"incubator" repo.

I'm happy to hold off on this PR until the new repo appears and service this for that?

@rhattersley
Copy link
Member

This is somewhat related to @bjlittle's upcoming "add-ons"/"extensions"/"incubator" repo.

I'm happy to hold off on this PR until the new repo appears and service this for that?

At the moment, I still think the main Iris repo is the place for this routine. I just wanted @bjlittle to be aware of this discussion since it's a related topic.

@rhattersley
Copy link
Member

Nudge... is there still life in this PR?

@cpelley
Copy link
Author

cpelley commented Apr 26, 2016

Hi @rhattersley thanks for prodding. Based on the feedback above (the limitations as well as fragilities, I decided to re-implement from an alternative approach yesterday, see #1983). This new implementation is also driven by a new requirement for in-place inversion without touching the data.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants