Skip to content
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

Shift changes non-float arrays to object, even for shift=0 #2451

Closed
max-sixty opened this issue Oct 1, 2018 · 2 comments
Closed

Shift changes non-float arrays to object, even for shift=0 #2451

max-sixty opened this issue Oct 1, 2018 · 2 comments

Comments

@max-sixty
Copy link
Collaborator

In [15]: xr.DataArray(np.random.randint(2,size=(100,100)).astype(bool)).shift(dim_0=0)
Out[15]:
<xarray.DataArray (dim_0: 100, dim_1: 100)>
array([[False, True, True, ..., True, True, False],
       [False, True, False, ..., False, True, True],
       [False, True, False, ..., False, True, False],
       ...,
       [False, True, False, ..., False, True, True],
       [True, False, True, ..., False, False, False],
       [False, True, True, ..., True, True, False]], dtype=object) # <-- could be bool
Dimensions without coordinates: dim_0, dim_1

Problem description

This causes memory bloat

Expected Output

As above with dtype=bool

Output of xr.show_versions()

In [16]: xr.show_versions()

INSTALLED VERSIONS

commit: f9c4169
python: 2.7.15.final.0
python-bits: 64
OS: Darwin
OS-release: 18.0.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: None.None

xarray: 0.10.9+12.gf9c41691
pandas: 0.22.0
numpy: 1.14.2
scipy: 1.0.0
netCDF4: None
h5netcdf: None
h5py: None
Nio: None
zarr: None
cftime: None
PseudonetCDF: None
rasterio: None
iris: None
bottleneck: 1.2.1
cyordereddict: None
dask: None
distributed: None
matplotlib: 2.1.2
cartopy: None
seaborn: 0.8.1
setuptools: 39.2.0
pip: 18.0
conda: None
pytest: 3.6.3
IPython: 5.8.0
sphinx: None

The shift=0 is mainly theoretical. To avoid casting to object in practical scenarios, we could add a fill_value argument (e.g. fill_value=False) and fill with that rather than NaN

CC @IvoCrnkovic

@shoyer
Copy link
Member

shoyer commented Oct 2, 2018

+1 for adding a fill_value argument to shift().

-1 for changing the behavior for shift=0 in particular. It's generally not a good idea to make the type/dtype of the result of operations dependent on the values of input arguments -- that sort of things can easily lead to bugs.

@max-sixty max-sixty mentioned this issue Oct 6, 2018
4 tasks
@max-sixty
Copy link
Collaborator Author

This was closed by #2470

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants