Skip to content

where ignores incorrectly typed arguments #3770

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

Closed
max-sixty opened this issue Feb 15, 2020 · 5 comments · Fixed by #3827
Closed

where ignores incorrectly typed arguments #3770

max-sixty opened this issue Feb 15, 2020 · 5 comments · Fixed by #3827

Comments

@max-sixty
Copy link
Collaborator

MCVE Code Sample

I optimistically tried passing a lambda to where* to avoid another .pipe. To my surprise it ran! But unfortunately it just ignored the lambda, as though all values were True.

In [1]: import xarray as xr                                                                                                                                         

In [2]: import numpy as np                                                                                                                                          

In [3]: da = xr.DataArray(np.random.rand(2,3))                                                                                                                      

In [4]: da.where(da > 0.5)                                                                                                                                          
Out[4]: 
<xarray.DataArray (dim_0: 2, dim_1: 3)>
array([[       nan, 0.71442406,        nan],
       [0.55748705,        nan,        nan]])
Dimensions without coordinates: dim_0, dim_1

# this should fail!

In [5]: da.where(lambda x: x > 0.5)                                                                                                                                 
Out[5]: 
<xarray.DataArray (dim_0: 2, dim_1: 3)>
array([[0.26085668, 0.71442406, 0.05816167],
       [0.55748705, 0.15293073, 0.12766453]])
Dimensions without coordinates: dim_0, dim_1

Expected Output

Raise a TypeError when passed the lambda

* (maybe that could work, but separate discussion)

Output of xr.show_versions()

INSTALLED VERSIONS ------------------ commit: None python: 3.6.8 (default, Aug 7 2019, 17:28:10) [...] python-bits: 64 OS: Linux OS-release: [...] machine: x86_64 processor: x86_64 byteorder: little LC_ALL: None LANG: en_US.utf8 LOCALE: en_US.UTF-8 libhdf5: 1.10.4 libnetcdf: None

xarray: 0.14.1
pandas: 0.25.3
numpy: 1.18.1
scipy: 1.4.1
netCDF4: None
pydap: None
h5netcdf: None
h5py: 2.10.0
Nio: None
zarr: None
cftime: None
nc_time_axis: None
PseudoNetCDF: None
rasterio: None
cfgrib: None
iris: None
bottleneck: None
dask: None
distributed: None
matplotlib: 3.1.2
cartopy: None
seaborn: 0.10.0
numbagg: None
setuptools: 45.0.0
pip: 20.0.2
conda: None
pytest: 5.3.2
IPython: 7.11.1
sphinx: 2.3.1

@max-sixty
Copy link
Collaborator Author

Would we want callables to be acceptable arguments to functions like where, assign, loc?

@shoyer
Copy link
Member

shoyer commented Feb 16, 2020

This is a NumPy thing, too:

In [6]: np.where(lambda: None, 1, 2)
Out[6]: array(1)

I think the problem is that functions cast to True with bool.

(I don't really understand why that is the case, but it does seem to be well-established behavior in Python)

@max-sixty
Copy link
Collaborator Author

👍

a) should we type check before we pass to numpy?
b) should we support single-argument callables?

@keewis
Copy link
Collaborator

keewis commented Feb 17, 2020

The docs state that the builtin bool returns True for all objects unless __bool__ is defined or __len__ returns 0 (function objects don't define either so they use the fallback). I'm not sure why, but I suspect it is the way it is to make

if obj:
    ...

equivalent to

if obj is not None:
    ...

That said I really like the callable support of pandas (e.g. Series.where), so maybe we should add that too?

@shoyer
Copy link
Member

shoyer commented Feb 17, 2020 via email

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

Successfully merging a pull request may close this issue.

3 participants