Skip to content

Conversation

@pelson
Copy link
Member

@pelson pelson commented Mar 30, 2017

The behaviour we are depending on is deprecated (and removed) in newer numpy versions...

$ python -c "import numpy as np; print(np.__version__); np.zeros(0.).dtype"
1.7.1

$ python -c "import numpy as np; print(np.__version__); np.zeros(0.)"
1.12.1
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'float' object cannot be interpreted as an index

We will also want to fix this in iris_grib.

@pelson
Copy link
Member Author

pelson commented Mar 30, 2017

Ping @djkirkham

@djkirkham
Copy link
Contributor

@pelson The documentation for np.zeros states that if the dtype is not given it defaults to np.float64. I suggest we just use that.

@pelson
Copy link
Member Author

pelson commented Apr 4, 2017

The documentation for np.zeros states that if the dtype is not given it defaults to np.float64. I suggest we just use that.

If we go that far, I'd go the whole hog and just use np.float_, or if it isn't designed to be architecture specific np.float64 (I don't know if it is supposed to depend on the arch or not, though I'd guess it does).

@marqh marqh added this to the v1.13.0 milestone May 11, 2017
@corinnebosley
Copy link
Member

@pelson I have been talking to Dan about this change. Based on our conversation, I think that this fix might be a bit more future-proof than the one in SciTools/iris-grib#75, and I want to merge this into master ready to be bundled into the upcoming v1.13.rc branch. How do you feel about that?

@corinnebosley
Copy link
Member

@pelson Please target this against master so that the changes can be carried forward into v1.13.x

@marqh
Copy link
Member

marqh commented May 15, 2017

removed from v1.13 milestone

I have created #2550 to get this change into the dask feature branch

the tests are not currently failing, running with numpy 1.12
this could be due to a lack of test coverage, I am not sure

I have also proposed adding this change to master, via #2551

@lbdreyer
Copy link
Member

I have also proposed adding this change to master, via #2551

It looks like it's a different commit in #2550 and #2551 (the commit sha's are different). Could you not have cherry picked the commit? I'm just concerned about the merge conflict it will create when it comes time to merge the dask branch back in.

@QuLogic
Copy link
Member

QuLogic commented May 16, 2017

#2551 is merged now.

Also, the commit SHAs must be different when cherry picked because the committer name and date are part of the hash.

@QuLogic QuLogic closed this May 16, 2017
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.

6 participants