-
-
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
WIP: indexing with broadcasting #1473
Changes from 1 commit
105bd64
23b4fe0
726ba5d
df7011f
f9232cb
17b6465
33c51d3
03a336f
d5af395
866de91
08e7444
84afc98
7b33269
50ea56e
bac0089
1206c28
c2747be
0671f39
ffccff1
becf539
1ae4b4c
1967bf5
c2eeff3
df12c04
5ba367d
d25c1f1
0115994
1b4e854
36d052f
9bd53ca
563cafa
1712060
884423a
c2e6f42
002eafa
eedfb3f
bb2e515
5983a67
7a5ff79
0b559bc
bad828e
a821a2b
6550880
464e711
7dd171d
e8f006b
a8ec82b
32749d4
31401d4
f42ddfd
8d96ad3
19f7204
a8f60ba
69f8570
5eb00b7
d133766
631f6e9
72587de
3231445
dd325c5
434a004
d518f7a
ba3cc88
f63f3d5
aa10635
fd73e82
6202aff
f9746fd
f78c932
1c027cd
d11829f
0777128
f580c99
4ebe852
20f5cb9
ab08af8
92dded6
a624424
a4cd724
f66c9b6
24309c4
fd698de
9cbaff9
a5c7766
f242166
bff18f0
21c11c4
1975f66
73ad94e
b49f813
1fd6b3a
46dd7c7
11f3e4f
1d3eddc
bcb25f1
7104964
173968b
addb91a
7ad7d36
91dd833
118a5d8
dc9f8a6
5726c89
523ecaa
a3a83db
765ae45
3deaf5c
c8c8a12
a16a04b
1b34cd4
24599a7
d5d967b
d0d6a6f
4f08e2e
fbbe35c
db23c93
031be9a
969f9cf
8608451
b4e5b36
9523039
dc60348
8a62ad9
6b96960
cb84154
9726531
caa79fe
170abc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,6 @@ | |
|
||
import numpy as np | ||
import pandas as pd | ||
import warnings | ||
import xarray as xr | ||
import pytest | ||
|
||
|
@@ -996,16 +995,16 @@ def test_isel_dataarray(self): | |
# Conflict in the dimension coordinate | ||
indexing_da = DataArray(np.arange(1, 4), dims=['dim2'], | ||
coords={'dim2': np.random.randn(3)}) | ||
with self.assertRaisesRegexp(IndexError, "Dimension coordinate dim2"): | ||
with self.assertRaisesRegexp( | ||
IndexError, "dimension coordinate 'dim2'"): | ||
actual = data.isel(dim2=indexing_da) | ||
# Also the case for DataArray | ||
with self.assertRaisesRegexp(IndexError, "Dimension coordinate dim2"): | ||
with self.assertRaisesRegexp( | ||
IndexError, "dimension coordinate 'dim2'"): | ||
actual = data['var2'].isel(dim2=indexing_da) | ||
|
||
# isel for the coordinate variable. Should not attach the coordinate | ||
actual = data['dim2'].isel(dim2=indexing_da) | ||
self.assertDataArrayIdentical(actual, | ||
data['dim2'].isel(dim2=np.arange(1, 4))) | ||
with self.assertRaisesRegexp( | ||
IndexError, "dimension coordinate 'dim2'"): | ||
data['dim2'].isel(dim2=indexing_da) | ||
|
||
# same name coordinate which does not conflict | ||
indexing_da = DataArray(np.arange(1, 4), dims=['dim2'], | ||
|
@@ -1055,13 +1054,13 @@ def test_isel_dataarray(self): | |
actual = data.isel(dim2=indexing_da) | ||
assert 'station' in actual | ||
actual = data.isel(dim2=indexing_da['station']) | ||
assert 'station' not in actual | ||
assert 'station' in actual | ||
|
||
# indexer generated from coordinates | ||
indexing_ds = Dataset({}, coords={'dim2': [0, 1, 2]}) | ||
actual = data.isel(dim2=indexing_ds['dim2']) | ||
expected = data.isel(dim2=[0, 1, 2]) | ||
self.assertDatasetIdentical(actual, expected) | ||
with self.assertRaisesRegexp( | ||
IndexError, "dimension coordinate 'dim2'"): | ||
actual = data.isel(dim2=indexing_ds['dim2']) | ||
|
||
def test_sel(self): | ||
data = create_test_data() | ||
|
@@ -1567,7 +1566,7 @@ def test_reindex_warning(self): | |
ind = xr.DataArray([0.0, 1.0], dims=['dim2'], name='ind') | ||
with pytest.warns(None) as ws: | ||
data.reindex(dim2=ind) | ||
assert len(ws) == 0 | ||
assert len(ws) == 0 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2 things here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not know a good way to make sure that the code does NOT warn. Does anyone know a better workaround? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this SO answer should work. ind = xr.DataArray([0.0, 1.0], dims=['dim2'], name='ind')
with pytest.warns(None) as ws:
# Should not warn
data.reindex(dim2=ind)
assert len(ws) == 0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. It now works without dummy warning. |
||
def test_reindex_variables_copied(self): | ||
data = create_test_data() | ||
|
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.
do we support indexers that are dask arrays? This would seem to load
v
into a numpy array.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.
Basically, in our current logic, we do not support dask indexer, although indexed arrays can be
dask
arrays.However, as you pointed out below, the use of
nonzero
method (slightly) relaxes this limitation, in some use cases,e.g. when the indexer is a large boolean array with only a few
True
entries.I want to do this (including to expose
Variable.nonzero
to public API), but I'm not sure whether it is not a pretty rare use case.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.
dask's
nonzero()
returns an array with unknown size. We should be sure we support that in xarray before we add nonzero to xarray.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.
To be clear, I'm not apposed to this implementation, provided we're all aware of how its going to behave.
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.
OK. I keep this line
and will update here after
Variable.nonzero()
is implemented.