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

xr.where() with numpy inputs produces error if global options changed #7362

Closed
4 tasks done
anissa111 opened this issue Dec 6, 2022 · 4 comments · Fixed by #7364
Closed
4 tasks done

xr.where() with numpy inputs produces error if global options changed #7362

anissa111 opened this issue Dec 6, 2022 · 4 comments · Fixed by #7364
Labels

Comments

@anissa111
Copy link

What happened?

#7229 seems to have introduced a possibility for errors if the set_options(keep_attrs=True) is used and then subsequently xr.where() is called using numpy inputs

What did you expect to happen?

Previous to xarray v2022.12.0, this did not occur. xr.where should have a check for non-Dataset inputs before trying to access attrs

Minimal Complete Verifiable Example

>>> import xarray as xr
>>> import numpy as np
>>> xr.set_options(keep_attrs=True)
<xarray.core.options.set_options object at 0x100cb8760>
>>> a = np.arange(10)
>>> b = np.arange(15,25)
>>> b = xr.where(a < 5, a, b)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/anissaz/opt/miniconda3/envs/geocat_comp_build/lib/python3.10/site-packages/xarray/core/computation.py", line 1887, in where
    result.attrs = getattr(x, "attrs", {})
AttributeError: 'numpy.ndarray' object has no attribute 'attrs'

MVCE confirmation

  • Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
  • Complete example — the example is self-contained, including all data and the text of any traceback.
  • Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
  • New issue — a search of GitHub Issues suggests this is not a duplicate.

Relevant log output

No response

Anything else we need to know?

Discovered via NCAR/geocat-comp#308

Environment

INSTALLED VERSIONS

commit: None
python: 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:41:54) [Clang 13.0.1 ]
python-bits: 64
OS: Darwin
OS-release: 21.6.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: ('en_US', 'UTF-8')
libhdf5: 1.12.2
libnetcdf: 4.8.1

xarray: 2022.12.0
pandas: 1.5.1
numpy: 1.23.5
scipy: 1.9.3
netCDF4: 1.6.2
pydap: None
h5netcdf: None
h5py: None
Nio: None
zarr: None
cftime: 1.6.2
nc_time_axis: None
PseudoNetCDF: None
rasterio: None
cfgrib: None
iris: None
bottleneck: 1.3.5
dask: 2022.10.0
distributed: 2022.10.0
matplotlib: 3.6.1
cartopy: 0.21.0
seaborn: None
numbagg: None
fsspec: 2022.10.0
cupy: None
pint: 0.19.2
sparse: None
flox: None
numpy_groupies: None
setuptools: 65.5.0
pip: 22.3
conda: None
pytest: 7.2.0
mypy: None
IPython: None
sphinx: None

@anissa111 anissa111 added bug needs triage Issue that has not been reviewed by xarray team member labels Dec 6, 2022
@dcherian
Copy link
Contributor

dcherian commented Dec 6, 2022

Thanks for the clear bug report!

b = xr.where(a < 5, a, b)

If the inputs are all numpy, you should be using np.where. We could raise a nicer error here.

But I'm not sure how it worked earlier. Des apply_ufunc return numpy output for all numpy inputs?

@dcherian dcherian removed the needs triage Issue that has not been reviewed by xarray team member label Dec 6, 2022
@anissa111
Copy link
Author

@dcherian That's a fair point. The function we wrote is supposed to support numpy and xarray input, and previously it was easier to use xr.where() for both cases instead of checking for input type repeatedly.

I just walked through my debugger with one of my failing tests and yes, it does appear that apply_ufunc is returning numpy output for all numpy input.

@slevang
Copy link
Contributor

slevang commented Dec 8, 2022

So many edge cases and different ways to use this function! I'm also a bit surprised numpy-only inputs work here, but if it does, we may as well support it. #7364 would be an easy way to handle this case.

@anissa111
Copy link
Author

Great, thank you! We're actually planning to removing the set_options that triggered the edge case here in geocat-comp, but I still think the check in #7364 is useful and might save someone else (possibly me again 😆 ) in the future some trouble.

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

Successfully merging a pull request may close this issue.

3 participants