From 997bec631f7a0c1cf581f4c362bdc461dc89feda Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 15:48:21 -0500 Subject: [PATCH 01/11] Better docs and errors about expand_dims() view --- xarray/core/dataarray.py | 4 +++- xarray/core/dataset.py | 3 ++- xarray/core/indexing.py | 10 +++++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index beaf148df6b..61c2f90ada8 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -1272,7 +1272,9 @@ def expand_dims(self, dim: Union[None, Hashable, Sequence[Hashable], Mapping[Hashable, Any]] = None, axis=None, **dim_kwargs: Any) -> 'DataArray': """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. + the corresponding position in the array shape. The new object is a + view into the underlying array, not a copy. + If dim is already a scalar coordinate, it will be promoted to a 1D coordinate consisting of a single value. diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 060c80b6722..59fb6468b9a 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2516,7 +2516,8 @@ def swap_dims(self, dims_dict, inplace=None): def expand_dims(self, dim=None, axis=None, **dim_kwargs): """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. + the corresponding position in the array shape. The new object is a + view into the underlying array, not a copy. If dim is already a scalar coordinate, it will be promoted to a 1D coordinate consisting of a single value. diff --git a/xarray/core/indexing.py b/xarray/core/indexing.py index 02953b74fa4..e262d9ee24b 100644 --- a/xarray/core/indexing.py +++ b/xarray/core/indexing.py @@ -1177,7 +1177,15 @@ def __getitem__(self, key): def __setitem__(self, key, value): array, key = self._indexing_array_and_key(key) - array[key] = value + try: + array[key] = value + except ValueError: + # More informative exception if read-only view + if not array.flags.writeable and not array.flags.owndata: + raise ValueError("Assignment destination is a view. " + "Do you want to .copy() array first?") + else: + raise class DaskIndexingAdapter(ExplicitlyIndexedNDArrayMixin): From 1de14207ea2bd9d0edb8f35f1d4e74da4e56efb4 Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:13:33 -0500 Subject: [PATCH 02/11] Add test and whatsnew note --- doc/whats-new.rst | 3 +++ xarray/tests/test_indexing.py | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index a1b3e5416ca..e0bde2f9b38 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -27,6 +27,9 @@ Enhancements Bug fixes ~~~~~~~~~ +- Improved error handling and documentation for `.expand_dims()` + read-only view. + .. _whats-new.0.12.3: v0.12.3 (10 July 2019) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index c044d2ed1f3..9c9a0b9e2df 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -144,6 +144,19 @@ def test_indexer(data, x, expected_pos, expected_idx=None): [True, True, True, True, False, False, False, False], pd.MultiIndex.from_product([[1, 2], [-1, -2]])) + def test_read_only_view(self): + arr = DataArray(np.random.rand(3,3), + coords={'x': np.arange(3), 'y': np.arange(3)}, + dims=('x', 'y')) # Create a 2D DataArray + arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + arr['z'] = np.arange(3) # New coordinates to dimension 'z' + try: + mess = "No exception?!" + arr.loc[0, 0, 0] = 999 + except ValueError as e: + mess = str(e) + assert "Do you want to .copy()" in mess + class TestLazyArray: def test_slice_slice(self): From 7fabdb97e6c8cbbf72180c543e47c8254c534dcc Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:15:37 -0500 Subject: [PATCH 03/11] Remove trailing whitespace --- xarray/core/dataarray.py | 2 +- xarray/core/dataset.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index 61c2f90ada8..6888e46fb18 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -1272,7 +1272,7 @@ def expand_dims(self, dim: Union[None, Hashable, Sequence[Hashable], Mapping[Hashable, Any]] = None, axis=None, **dim_kwargs: Any) -> 'DataArray': """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. The new object is a + the corresponding position in the array shape. The new object is a view into the underlying array, not a copy. diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 59fb6468b9a..71197026aa2 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2516,7 +2516,7 @@ def swap_dims(self, dims_dict, inplace=None): def expand_dims(self, dim=None, axis=None, **dim_kwargs): """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. The new object is a + the corresponding position in the array shape. The new object is a view into the underlying array, not a copy. If dim is already a scalar coordinate, it will be promoted to a 1D From 8d170395edb835ca813e00d1628a6e5ec93d5236 Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:18:32 -0500 Subject: [PATCH 04/11] Whitespace for PEP8 --- xarray/tests/test_indexing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index 9c9a0b9e2df..2fb8cbe04c9 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -145,11 +145,11 @@ def test_indexer(data, x, expected_pos, expected_idx=None): pd.MultiIndex.from_product([[1, 2], [-1, -2]])) def test_read_only_view(self): - arr = DataArray(np.random.rand(3,3), - coords={'x': np.arange(3), 'y': np.arange(3)}, - dims=('x', 'y')) # Create a 2D DataArray - arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' - arr['z'] = np.arange(3) # New coordinates to dimension 'z' + arr = DataArray(np.random.rand(3, 3), + coords={'x': np.arange(3), 'y': np.arange(3)}, + dims=('x', 'y')) # Create a 2D DataArray + arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + arr['z'] = np.arange(3) # New coords to dimension 'z' try: mess = "No exception?!" arr.loc[0, 0, 0] = 999 From 1bbc21585cc78e3a77281b19ad6694673f7ba3e4 Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:50:51 -0500 Subject: [PATCH 05/11] Py35 compatible test --- xarray/tests/test_indexing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index 2fb8cbe04c9..12c41b27bba 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -145,10 +145,12 @@ def test_indexer(data, x, expected_pos, expected_idx=None): pd.MultiIndex.from_product([[1, 2], [-1, -2]])) def test_read_only_view(self): + from collections import OrderedDict arr = DataArray(np.random.rand(3, 3), coords={'x': np.arange(3), 'y': np.arange(3)}, dims=('x', 'y')) # Create a 2D DataArray - arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + + arr = arr.expand_dims(OrderedDict([('z', 3)]), -1) # New dimension 'z' arr['z'] = np.arange(3) # New coords to dimension 'z' try: mess = "No exception?!" From e5ad6c0c5382ce6c433426ffbff770a6d41a11cc Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 15:48:21 -0500 Subject: [PATCH 06/11] Better docs and errors about expand_dims() view --- xarray/core/dataarray.py | 4 +++- xarray/core/dataset.py | 3 ++- xarray/core/indexing.py | 10 +++++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index beaf148df6b..61c2f90ada8 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -1272,7 +1272,9 @@ def expand_dims(self, dim: Union[None, Hashable, Sequence[Hashable], Mapping[Hashable, Any]] = None, axis=None, **dim_kwargs: Any) -> 'DataArray': """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. + the corresponding position in the array shape. The new object is a + view into the underlying array, not a copy. + If dim is already a scalar coordinate, it will be promoted to a 1D coordinate consisting of a single value. diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 060c80b6722..59fb6468b9a 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2516,7 +2516,8 @@ def swap_dims(self, dims_dict, inplace=None): def expand_dims(self, dim=None, axis=None, **dim_kwargs): """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. + the corresponding position in the array shape. The new object is a + view into the underlying array, not a copy. If dim is already a scalar coordinate, it will be promoted to a 1D coordinate consisting of a single value. diff --git a/xarray/core/indexing.py b/xarray/core/indexing.py index 02953b74fa4..e262d9ee24b 100644 --- a/xarray/core/indexing.py +++ b/xarray/core/indexing.py @@ -1177,7 +1177,15 @@ def __getitem__(self, key): def __setitem__(self, key, value): array, key = self._indexing_array_and_key(key) - array[key] = value + try: + array[key] = value + except ValueError: + # More informative exception if read-only view + if not array.flags.writeable and not array.flags.owndata: + raise ValueError("Assignment destination is a view. " + "Do you want to .copy() array first?") + else: + raise class DaskIndexingAdapter(ExplicitlyIndexedNDArrayMixin): From 3828e94af30632319beb3b2ed752199c1489a2eb Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:13:33 -0500 Subject: [PATCH 07/11] Add test and whatsnew note --- doc/whats-new.rst | 3 +++ xarray/tests/test_indexing.py | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index a1b3e5416ca..e0bde2f9b38 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -27,6 +27,9 @@ Enhancements Bug fixes ~~~~~~~~~ +- Improved error handling and documentation for `.expand_dims()` + read-only view. + .. _whats-new.0.12.3: v0.12.3 (10 July 2019) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index c044d2ed1f3..9c9a0b9e2df 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -144,6 +144,19 @@ def test_indexer(data, x, expected_pos, expected_idx=None): [True, True, True, True, False, False, False, False], pd.MultiIndex.from_product([[1, 2], [-1, -2]])) + def test_read_only_view(self): + arr = DataArray(np.random.rand(3,3), + coords={'x': np.arange(3), 'y': np.arange(3)}, + dims=('x', 'y')) # Create a 2D DataArray + arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + arr['z'] = np.arange(3) # New coordinates to dimension 'z' + try: + mess = "No exception?!" + arr.loc[0, 0, 0] = 999 + except ValueError as e: + mess = str(e) + assert "Do you want to .copy()" in mess + class TestLazyArray: def test_slice_slice(self): From b65bff6fcb47070afa063254cbfa23b2bbd73489 Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:15:37 -0500 Subject: [PATCH 08/11] Remove trailing whitespace --- xarray/core/dataarray.py | 2 +- xarray/core/dataset.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/xarray/core/dataarray.py b/xarray/core/dataarray.py index 61c2f90ada8..6888e46fb18 100644 --- a/xarray/core/dataarray.py +++ b/xarray/core/dataarray.py @@ -1272,7 +1272,7 @@ def expand_dims(self, dim: Union[None, Hashable, Sequence[Hashable], Mapping[Hashable, Any]] = None, axis=None, **dim_kwargs: Any) -> 'DataArray': """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. The new object is a + the corresponding position in the array shape. The new object is a view into the underlying array, not a copy. diff --git a/xarray/core/dataset.py b/xarray/core/dataset.py index 59fb6468b9a..71197026aa2 100644 --- a/xarray/core/dataset.py +++ b/xarray/core/dataset.py @@ -2516,7 +2516,7 @@ def swap_dims(self, dims_dict, inplace=None): def expand_dims(self, dim=None, axis=None, **dim_kwargs): """Return a new object with an additional axis (or axes) inserted at - the corresponding position in the array shape. The new object is a + the corresponding position in the array shape. The new object is a view into the underlying array, not a copy. If dim is already a scalar coordinate, it will be promoted to a 1D From 9c77b31109287fc8c7772261c4fd167c3c85290c Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:18:32 -0500 Subject: [PATCH 09/11] Whitespace for PEP8 --- xarray/tests/test_indexing.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index 9c9a0b9e2df..2fb8cbe04c9 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -145,11 +145,11 @@ def test_indexer(data, x, expected_pos, expected_idx=None): pd.MultiIndex.from_product([[1, 2], [-1, -2]])) def test_read_only_view(self): - arr = DataArray(np.random.rand(3,3), - coords={'x': np.arange(3), 'y': np.arange(3)}, - dims=('x', 'y')) # Create a 2D DataArray - arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' - arr['z'] = np.arange(3) # New coordinates to dimension 'z' + arr = DataArray(np.random.rand(3, 3), + coords={'x': np.arange(3), 'y': np.arange(3)}, + dims=('x', 'y')) # Create a 2D DataArray + arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + arr['z'] = np.arange(3) # New coords to dimension 'z' try: mess = "No exception?!" arr.loc[0, 0, 0] = 999 From ef09c6e2cc89f3e6a3787ac5029b40bf822b21f8 Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sat, 13 Jul 2019 16:50:51 -0500 Subject: [PATCH 10/11] Py35 compatible test --- xarray/tests/test_indexing.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index 2fb8cbe04c9..12c41b27bba 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -145,10 +145,12 @@ def test_indexer(data, x, expected_pos, expected_idx=None): pd.MultiIndex.from_product([[1, 2], [-1, -2]])) def test_read_only_view(self): + from collections import OrderedDict arr = DataArray(np.random.rand(3, 3), coords={'x': np.arange(3), 'y': np.arange(3)}, dims=('x', 'y')) # Create a 2D DataArray - arr = arr.expand_dims({'z': 3}, -1) # Add a new dimension 'z' + + arr = arr.expand_dims(OrderedDict([('z', 3)]), -1) # New dimension 'z' arr['z'] = np.arange(3) # New coords to dimension 'z' try: mess = "No exception?!" From 0037351c3b4ec9fa38ceca437680ec41b454ec6a Mon Sep 17 00:00:00 2001 From: David Mertz Date: Sun, 14 Jul 2019 09:13:18 -0500 Subject: [PATCH 11/11] Improved exception testing --- xarray/tests/test_indexing.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/xarray/tests/test_indexing.py b/xarray/tests/test_indexing.py index 12c41b27bba..64eee80d4eb 100644 --- a/xarray/tests/test_indexing.py +++ b/xarray/tests/test_indexing.py @@ -149,15 +149,10 @@ def test_read_only_view(self): arr = DataArray(np.random.rand(3, 3), coords={'x': np.arange(3), 'y': np.arange(3)}, dims=('x', 'y')) # Create a 2D DataArray - arr = arr.expand_dims(OrderedDict([('z', 3)]), -1) # New dimension 'z' arr['z'] = np.arange(3) # New coords to dimension 'z' - try: - mess = "No exception?!" + with pytest.raises(ValueError, match='Do you want to .copy()'): arr.loc[0, 0, 0] = 999 - except ValueError as e: - mess = str(e) - assert "Do you want to .copy()" in mess class TestLazyArray: