-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Coordinates passed to interp have nan values #3924
Conversation
Hi @zxdawn thanks for the PR, appreciate you making a first contribution. I don't know this code that well, but your examples look reasonable. Any thoughts from those that know this better, @spencerkclark @huard @dcherian ? If we do go ahead and merge this solution, we'd need tests @zxdawn , would you be up for writing those? |
Hi @max-sixty, thanks. |
Indeed, thanks @zxdawn, this seems like a reasonable change to me too. I say go ahead with writing the tests. |
Note that it looks like this causes problems for older NumPy versions when interpolating with a datetime coordinate, e.g. in the Linux py36-min-nep18 build:
It looks like support for |
@spencerkclark Maybe converting the datetime into number? BTW, Why not forcing numpy >= 1.18.1? |
imin = index.get_loc(np.nanmin(new_x.values), method="nearest") | ||
imax = index.get_loc(np.nanmax(new_x.values), method="nearest") |
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.
At first I was a little uneasy about your suggestion to require a newer version of NumPy, but after reading about the history of this issue a little more, I think something along those lines may actually be the simplest approach (perhaps you were ahead of me on this).
It turns out that np.min
and np.max
behave like np.nanmin
and np.nanmax
for time-like types in versions of NumPy prior to 1.18.1 (see numpy/numpy#14841 (comment)), so we can continue to use those for time-like types and earlier versions of NumPy and still obtain the desired behavior. I think something like the following would work:
imin = index.get_loc(np.nanmin(new_x.values), method="nearest") | |
imax = index.get_loc(np.nanmax(new_x.values), method="nearest") | |
if LooseVersion(np.__version__) < LooseVersion("1.18.1") and new_x.dtype.kind in "mM": | |
# In older versions of NumPy min and max (incorrectly) ignore NaT values | |
# for time-like types -- that is they behave like nanmin and nanmax | |
# should -- but nanmin and nanmax raise an error. | |
imin = index.get_loc(np.min(new_x.values), method="nearest") | |
imax = index.get_loc(np.max(new_x.values), method="nearest") | |
else: | |
imin = index.get_loc(np.nanmin(new_x.values), method="nearest") | |
imax = index.get_loc(np.nanmax(new_x.values), method="nearest") |
It's my first time to write
How to make the test actually run? |
Do you have scipy installed? |
@dcherian Oh, thanks! After
I will update the test and pull it soon. |
Thank you for the PR - this now fixed in #4233 |
Problem
Keyerror
when the coordinates passed tointerp
have nan value.Example
Output
Solution
Use
np.nanmin
andnp.nanmax
inindex.get_loc()
Test
Code
Output