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
numpy 1.8 compatibility for NAN functions #150
Conversation
So did other functions change too? As in, do we need to include all these here, or can we import some from any version of numpy? |
Good question. I had just assumed they all changed. |
Maybe you could just do a diff with the 1.9 version? |
As far as I can tell, only nansum, nanmin, nanmax changed:
|
I guess numpy 1.7 had nanmean etc. in a different place? I'll have to hunt that down tomorrow. |
Argh. This approach is no good. |
@astrofrog @ChrisBeaumont I should probably squash this, but what do you think about this solution? It shouldn't impact performance except for numpy >= 1.9, and there only minimally |
from 1.8-1.9 add numpy 1.8 nanfunctions to ensure np1.8 = np1.9 behavior udpate tests moments no longer uses np.nansum (np_compat.nansum instead) remove all but nansum, nanmin, nanmax import things that needed to be imported (oops) correct imports and add support functions for numpy 1.7. If this fails on 1.6, 3.2, I suggest we drop support for them completely copying over numpy functions was way too much and would have lead to a huge maintenance burden. Instead, just overload nansum and leave the rest untouched remove np_compat; we're just going to change the tests remove np_compat 1.9 nanish override; everything else gets to stay the same
@@ -161,7 +162,11 @@ def moment_cubewise(cube, order, axis): | |||
data = cube._get_filled_data() * cube._pix_size()[axis] | |||
|
|||
if order == 0: | |||
return np.nansum(data, axis=axis) | |||
result = np.nansum(data, axis=axis) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would package this into a custom nansum utility function, for better separation of concerns.
numpy 1.8 compatibility for NAN functions
Fixes the issue noted in #149 thanks to @astrofrog's solution to the SO question.