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

DataArray groupby returning Dataset broken in some cases #6393

Closed
aulemahal opened this issue Mar 21, 2022 · 1 comment · Fixed by #6394
Closed

DataArray groupby returning Dataset broken in some cases #6393

aulemahal opened this issue Mar 21, 2022 · 1 comment · Fixed by #6394

Comments

@aulemahal
Copy link
Contributor

What happened?

This is a the reverse problem of #6379, the DataArrayGroupBy._combine method seems broken when the mapped function returns a Dataset (which worked before #5692).

What did you expect to happen?

No response

Minimal Complete Verifiable Example

import xarray as xr

ds = xr.tutorial.open_dataset("air_temperature")

ds.air.resample(time="YS").map(lambda grp: grp.mean("time").to_dataset())

Relevant log output

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [3], in <module>
----> 1 ds.air.resample(time="YS").map(lambda grp: grp.mean("time").to_dataset())

File ~/Python/myxarray/xarray/core/resample.py:223, in DataArrayResample.map(self, func, shortcut, args, **kwargs)
    180 """Apply a function to each array in the group and concatenate them
    181 together into a new array.
    182 
   (...)
    219     The result of splitting, applying and combining this array.
    220 """
    221 # TODO: the argument order for Resample doesn't match that for its parent,
    222 # GroupBy
--> 223 combined = super().map(func, shortcut=shortcut, args=args, **kwargs)
    225 # If the aggregation function didn't drop the original resampling
    226 # dimension, then we need to do so before we can rename the proxy
    227 # dimension we used.
    228 if self._dim in combined.coords:

File ~/Python/myxarray/xarray/core/groupby.py:835, in DataArrayGroupByBase.map(self, func, shortcut, args, **kwargs)
    833 grouped = self._iter_grouped_shortcut() if shortcut else self._iter_grouped()
    834 applied = (maybe_wrap_array(arr, func(arr, *args, **kwargs)) for arr in grouped)
--> 835 return self._combine(applied, shortcut=shortcut)

File ~/Python/myxarray/xarray/core/groupby.py:869, in DataArrayGroupByBase._combine(self, applied, shortcut)
    867     index, index_vars = create_default_index_implicit(coord)
    868     indexes = {k: index for k in index_vars}
--> 869     combined = combined._overwrite_indexes(indexes, coords=index_vars)
    870 combined = self._maybe_restore_empty_groups(combined)
    871 combined = self._maybe_unstack(combined)

TypeError: _overwrite_indexes() got an unexpected keyword argument 'coords'

Anything else we need to know?

I guess the same solution as #6386 could be used!

Environment

INSTALLED VERSIONS

commit: None
python: 3.9.6 | packaged by conda-forge | (default, Jul 11 2021, 03:39:48)
[GCC 9.3.0]
python-bits: 64
OS: Linux
OS-release: 5.16.13-arch1-1
machine: x86_64
processor:
byteorder: little
LC_ALL: None
LANG: fr_CA.utf8
LOCALE: ('fr_CA', 'UTF-8')
libhdf5: 1.12.0
libnetcdf: 4.7.4

xarray: 2022.3.1.dev16+g3ead17ea
pandas: 1.4.0
numpy: 1.20.3
scipy: 1.7.1
netCDF4: 1.5.7
pydap: None
h5netcdf: 0.11.0
h5py: 3.4.0
Nio: None
zarr: 2.10.0
cftime: 1.5.0
nc_time_axis: 1.3.1
PseudoNetCDF: None
rasterio: None
cfgrib: None
iris: None
bottleneck: 1.3.2
dask: 2021.08.0
distributed: 2021.08.0
matplotlib: 3.4.3
cartopy: None
seaborn: None
numbagg: None
fsspec: 2021.07.0
cupy: None
pint: 0.18
sparse: None
setuptools: 57.4.0
pip: 21.2.4
conda: None
pytest: 6.2.5
IPython: 8.0.1
sphinx: 4.1.2

@aulemahal aulemahal added bug needs triage Issue that has not been reviewed by xarray team member labels Mar 21, 2022
@benbovy
Copy link
Member

benbovy commented Mar 21, 2022

Ah yes, good catch @aulemahal. I'd be surprised if such example of returning a Dataset from DataArray groups is a common case, but since it seems to work well there no reason not to support it?

-> #6394

@benbovy benbovy added topic-indexing and removed needs triage Issue that has not been reviewed by xarray team member labels Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants