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

BUG: Iris coords bounds indexing broken with multiple ellipsis #2425

Closed
cpelley opened this issue Mar 10, 2017 · 16 comments
Closed

BUG: Iris coords bounds indexing broken with multiple ellipsis #2425

cpelley opened this issue Mar 10, 2017 · 16 comments

Comments

@cpelley
Copy link

cpelley commented Mar 10, 2017

In numpy v1.12.0 it is no longer simply deprecated to index with a double ellipsis, it is now enforced by an IndexError exception raised. Here is the relevant numpy change.

This exposes a bug in iris as iris.util.column_slices_generator can return an ellipsis, then an additional ellipsis is also appended here.

>>> import numpy as np
>>> import iris.coords.DimCoord as DimCoord
>>> np.__version__
'1.12.0'
>>> coord = DimCoord([-1,  0,  1,  2])
>>> coord.guess_bounds()
>>> coord[np.array([0, 3])]
*** IndexError: an index can only have a single ellipsis ('...')

Here is a partial traceback:

...
    coord = super(DimCoord, self).__getitem__(key)
  File "/path/to/iris/coords.py", line 496, in __getitem__
    bounds = bounds[keys + (Ellipsis, )]
IndexError: an index can only have a single ellipsis ('...')

Workaround is to explicitly specify numpy as one of our dependencies and pin its version to 1.11

@cpelley
Copy link
Author

cpelley commented Mar 10, 2017

This exposes a problem with the travis testing. I notice that the iris travis tests still appear to be using numpy v1.10 but I get numpy v1.12 (the latest) via a conda install iris with the conda forge channel (where one get's iris v1.12 right now). Since iris supports later versions of numpy, I think this exposes a bug and a frailty of testing here.

@marqh
Copy link
Member

marqh commented Mar 10, 2017

this is being actively worked on by myself and @djkirkham

i know there are changes staged for the coord API to avoid double ellipsis
suggestions for increasing test coverage in this realm are most welcome, please notify @djkirkham and I of these

@cpelley
Copy link
Author

cpelley commented Mar 10, 2017

this is being actively worked on by myself and @djkirkham

Great news, let me know if I can help in any way.

@marqh
Copy link
Member

marqh commented Mar 10, 2017

Great news, let me know if I can help in any way.

writing a test case for us which catches the failure mode you are interested in would help

the only failure i remember seeing was fixed inside iris.coord not iris.util

but this is exactly here:

  File "/path/to/iris/coords.py", line 496, in __getitem__
    bounds = bounds[keys + (Ellipsis, )]

iirc

@cpelley
Copy link
Author

cpelley commented Mar 10, 2017

the only failure i remember seeing was fixed inside iris.coord not iris.util

That's right the failure is indeed in iris.coord. I mentioned iris.util.column_slices_generator only as context to where the two ellipsis had come from that's all.

@cpelley
Copy link
Author

cpelley commented Mar 27, 2017

this is being actively worked on by myself and @djkirkham

@marqh do you have an open PR/issue?

writing a test case for us which catches the failure mode you are interested in would help

See the ticket description. Happy to turn this into a failing unittest for your branch if you want.

@DamienIrving
Copy link

Here's an error I got today which might be relevant to this bug (and might be a good test case):

iris.coord_categorisation.add_year(cube, 'time')
cube = cube.aggregated_by(['year'], iris.analysis.MEAN)

IndexError: an index can only have a single ellipsis ('...')

@marqh
Copy link
Member

marqh commented Apr 4, 2017

I believe #2440 has fixed this issue, as part of a review of version dependencies

@cpelley @DamienIrving
if you get an opportunity to confirm this, or raise why it is still an issue, that would be helpful

@DamienIrving
Copy link

DamienIrving commented Apr 5, 2017

@marqh To confirm I'd need to install iris from source, right? I usually install using conda and have very little sys admin experience, so what would be the easiest way for me to test the source? (Or will the changes made in #2440 be part of the conda install soon anyway?)

@cpelley
Copy link
Author

cpelley commented Apr 5, 2017

I'll confirm shortly, just have numpy building. @DamienIrving, if you can provide me a small isolated example to run, I can see if I can run it for you?

@cpelley
Copy link
Author

cpelley commented Apr 5, 2017

Confirmed, example in the ticket description now works.

Cheers

@DamienIrving
Copy link

@cpelley If you have any cube with a time axis and try the following you'll get the error:

iris.coord_categorisation.add_year(cube, 'time')
cube = cube.aggregated_by(['year'], iris.analysis.MEAN)

IndexError: an index can only have a single ellipsis ('...')

@cpelley
Copy link
Author

cpelley commented Apr 5, 2017

@DamienIrving I can confirm the following works for me now with iris master:

>>> import iris.tests.stock
>>> import iris.coord_categorisation
>>> import numpy as np
>>> np.__version__
'1.12.1'
>>> cube = iris.tests.stock.realistic_3d()
>>> iris.coord_categorisation.add_year(cube, 'time')
>>> cube.aggregated_by(['year'], iris.analysis.MEAN)
<iris 'Cube' of air_potential_temperature / (K) (time: 1; grid_latitude: 9; grid_longitude: 11)>

You happy for me to close this issue?

@DamienIrving
Copy link

@cpelley Thanks! I'd be happy for you to close the issue.

Do you know how long it might be before this change is available to those who install iris via conda? The inability to create an annual mean timeseries is rather crippling for many of my workflows.

@DamienIrving
Copy link

DamienIrving commented Apr 6, 2017

@cpelley I found a good solution - I simply used conda to downgrade the version of numpy currently installed:

$ conda install numpy=1.11.3

@cpelley
Copy link
Author

cpelley commented Apr 6, 2017

Do you know how long it might be before this change is available to those who install iris via conda?

I'm afraid I haven't heard any talk of a new point release in the works. iris v2 release appears to be the main drive right now which is hoped for ~June. @SciTools/iris-devs can correct me if I'm wrong.

I simply used conda to downgrade the version of numpy currently installed

Yes our project does the same :)

@cpelley cpelley closed this as completed Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants