-
-
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
expand_dims() modifies numpy.ndarray.flags to write only, upon manually reverting this flag back, attempting to set a single inner value using .loc will instead set all of the inner array values #2891
Comments
As you've noticed, these arrays are "read only" because otherwise indexing can modify more than the original values, e.g., consider: original = xr.DataArray(np.zeros(3), dims='x')
result = original.expand_dims(y=2)
result.data.flags.writeable = True
result[0, 0] = 1
print(result) Both "y" values were set to 1!
The work around is to call
Now the correct result is printed:
This is indeed intended behavior: by making the result read-only, we can expand dimensions without copying the original data, and without needing to worry about indexing modifying the wrong values. That said, we could certainly improve the usability of this feature in xarray. Some options:
|
Thank you @shoyer for taking the time to educate me on this. I understand completely now. I agree that solutions one and two would be helpful for future developers new to Xarray and the expand_dims operation when they eventually encounter this behaviour. I also agree that option three would be confusing, and had it of been implemented as such I would still have found myself to be asking a similar question about why it is like that. Another option to consider which might be easier still would be to just update the expand_dims documentation to include a note about this behaviour and the copy solution. Thanks again! |
OK, we would definitely welcome a pull request to improve this error message and the documentation for I lean slightly against adding the |
Yes good point. I do also think that the expand_dims interface should really only be responsible for that one single operation. If you then want to make a copy then go ahead and use that separate method afterwards. Ok great, if I get time later today I'll see if I can't pick that up; that is if someone hasn't already done so in the meantime. |
I am also affected in some code which used to work with earlier versions of xarray. In this case, I call
The |
I have just realized that |
I am using the newly updated expand_dims API that was recently updated with this PR #2757. However the flag setting behaviour can also be observed using the old API syntax.
Problem description
When applying the operation 'expand_dims({'z': 3}, -1)' on a DataArray the underlying Numpy array flags are changed. 'C_CONTIGUOUS' is set to False, and 'WRITEABLE' is set to False, and 'OWNDATA' is set to False. Upon changing 'WRITEABLE' back to True, when I try to set a single value in the DataArray using the '.loc' operator it will instead set all the values in that selected inner array.
I am new to Xarray so I can't be entirely sure if this expected behaviour. Regardless I would expect that adding a new dimension to the array would not make that array 'read-only'. I would also not expect the '.loc' method to work differently to how it would otherwise.
It's also not congruent with the Numpy 'expand_dims' operation. Because when I call the operation 'np.expand_dims(np_arr, axis=-1)' the 'C_CONTIGUOUS ' and 'WRITEABLE ' flags will not be modified.
Expected Output
Here is a similar flow of operations that demonstrates the behaviour I would expect from the DataArray after applying 'expand_dims':
Output of
xr.show_versions()
xarray: 0.12.1
pandas: 0.24.2
numpy: 1.16.2
scipy: 1.2.1
netCDF4: 1.5.0
pydap: None
h5netcdf: None
h5py: None
Nio: None
zarr: None
cftime: 1.0.3.4
nc_time_axis: None
PseudonetCDF: None
rasterio: None
cfgrib: 0.9.6.1.post1
iris: None
bottleneck: None
dask: None
distributed: None
matplotlib: 3.0.3
cartopy: 0.17.0
seaborn: None
setuptools: 39.0.1
pip: 10.0.1
conda: None
pytest: None
IPython: None
sphinx: None
The text was updated successfully, but these errors were encountered: