interp() now accepts date strings as desired co-ordinate locations #2325
Conversation
xarray/core/dataset.py
Outdated
| @@ -1328,6 +1330,12 @@ def _validate_indexers(self, indexers): | |||
| raise TypeError('cannot use a Dataset as an indexer') | |||
| else: | |||
| v = np.asarray(v) | |||
|
|
|||
| if (v.dtype.kind == 'U' | |||
There was a problem hiding this comment.
only the first half is really required (the tests all pass), but I added the second bit to be safe.
|
Thanks, @dcherian . It looks already great, but can you add a bit more test cases to keep it working with the future modification?
I think they will be passing with the current PR though. |
|
Can you merge master? It will solve some failures in CI. |
* master: Add libraries to show_versions (pydata#2321) fix doc build error after pydata#2312 (pydata#2326)
xarray/tests/test_interp.py
Outdated
| (np.array([np.datetime64('2000-01-01T12:00'), | ||
| np.datetime64('2000-01-02T12:00')]), [0.5, 1.5]), | ||
| (['2000-01-01T12:00', '2000-01-02T12:00'], [0.5, 1.5]), | ||
| (['2000-01-01T12:00'], 0.5) # needs to be list, see #2327 |
There was a problem hiding this comment.
Can you remove the comment and add a test with a string (not a list)?
There was a problem hiding this comment.
It looks it would still be a good idea to fix the scalar case?
| @@ -462,19 +462,30 @@ def test_interp_like(): | |||
|
|
|||
|
|
|||
| @requires_scipy | |||
| def test_datetime(): | |||
| da = xr.DataArray(np.random.randn(24), dims='time', | |||
| @pytest.mark.parametrize('x_new, expected', [ | |||
There was a problem hiding this comment.
I think requires_scipy does not work with another decorator. (We need to improve it though)
A workaround is to pytest.importorskip('scipy') inside the function.
There was a problem hiding this comment.
I forgot to add '@requires_scipy' with the new test
There was a problem hiding this comment.
OK. Forget my comment. I will be merging it later.
|
Thanks, @dcherian. merging. |
|
|
||
|
|
||
| @requires_scipy | ||
| def test_datetime_single_string(): |
There was a problem hiding this comment.
@shoyer the scalar case works and is tested here. I didn't want to make the previous test too complicated (it fails because the result of the scalar case has no dims; but the test checks both dims and coords).
whats-new.rstfor all changes andapi.rstfor new API (remove if this change should not be visible to users, e.g., if it is an internal clean-up, or if this is part of a larger project that will be documented later)