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

Groupby reduction fails when all groups are of size 1 #8518

Closed
5 tasks done
siranipour opened this issue Dec 4, 2023 · 3 comments · Fixed by #8507
Closed
5 tasks done

Groupby reduction fails when all groups are of size 1 #8518

siranipour opened this issue Dec 4, 2023 · 3 comments · Fixed by #8507

Comments

@siranipour
Copy link

siranipour commented Dec 4, 2023

What happened?

When grouping by a dimension with no duplicate coordinates, reduction operations fail. Reductions don't fail if there are duplicate coordinates, or if an empty tuple is provided in the reduction operation.

What did you expect to happen?

Reductions happening after a groupby operation which leads to each group being a singleton should not fail. By default it should mimic the same behaviour as when an empty tuple is passed to the reduction operation.

Minimal Complete Verifiable Example

import numpy as np
import xarray as xr

a = xr.DataArray(range(3), coords={'a': [0, 1, 1]})
b = xr.DataArray(range(3), coords={'a': [0, 1, 2]})

# Works completely fine
a.groupby('a').sum()

# What is expected from the below:
b.groupby('a').sum(tuple())

# Raises value error
b.groupby('a').sum()

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.
  • Recent environment — the issue occurs with the latest version of xarray and its dependencies.

Relevant log output

ValueError                                Traceback (most recent call last)
File /private/tmp/foo.py:11, in <module>
      8 a.groupby('a').sum()
     10 # Raises value error
---> 11 b.groupby('a').sum()
     13 # What is expected from the above:
     14 b.groupby('a').sum(tuple())

File ~/miniconda3/lib/python3.10/site-packages/xarray/core/_aggregations.py:6204, in DataArrayGroupByAggregations.sum(self, dim, skipna, min_count, keep_attrs, **kwargs)
   6194     return self._flox_reduce(
   6195         func="sum",
   6196         dim=dim,
   (...)
   6201         **kwargs,
   6202     )
   6203 else:
-> 6204     return self.reduce(
   6205         duck_array_ops.sum,
   6206         dim=dim,
   6207         skipna=skipna,
   6208         min_count=min_count,
   6209         keep_attrs=keep_attrs,
   6210         **kwargs,
   6211     )

File ~/miniconda3/lib/python3.10/site-packages/xarray/core/groupby.py:1478, in DataArrayGroupByBase.reduce(self, func, dim, axis, keep_attrs, keepdims, shortcut, **kwargs)
   1468 def reduce_array(ar: DataArray) -> DataArray:
   1469     return ar.reduce(
   1470         func=func,
   1471         dim=dim,
   (...)
   1475         **kwargs,
   1476     )
-> 1478 check_reduce_dims(dim, self.dims)
   1480 return self.map(reduce_array, shortcut=shortcut)

File ~/miniconda3/lib/python3.10/site-packages/xarray/core/groupby.py:69, in check_reduce_dims(reduce_dims, dimensions)
     67     reduce_dims = [reduce_dims]
     68 if any(dim not in dimensions for dim in reduce_dims):
---> 69     raise ValueError(
     70         f"cannot reduce over dimensions {reduce_dims!r}. expected either '...' "
     71         f"to reduce over all dimensions or one or more of {dimensions!r}."
     72     )

ValueError: cannot reduce over dimensions ['a']. expected either '...' to reduce over all dimensions or one or more of ().

Anything else we need to know?

No response

Environment

INSTALLED VERSIONS

commit: None
python: 3.10.5 | packaged by conda-forge | (main, Jun 14 2022, 07:03:09) [Clang 13.0.1 ]
python-bits: 64
OS: Darwin
OS-release: 23.0.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_GB.UTF-8
LOCALE: ('en_GB', 'UTF-8')
libhdf5: 1.12.1
libnetcdf: None

xarray: 2023.6.0
pandas: 1.5.3
numpy: 1.23.5
scipy: 1.8.1
netCDF4: None
pydap: None
h5netcdf: None
h5py: 3.6.0
Nio: None
zarr: None
cftime: None
nc_time_axis: None
PseudoNetCDF: None
iris: None
bottleneck: 1.3.5
dask: 2022.8.0
distributed: 2022.8.0
matplotlib: 3.5.2
cartopy: None
seaborn: None
numbagg: None
fsspec: 2023.4.0
cupy: None
pint: None
sparse: None
flox: None
numpy_groupies: None
setuptools: 67.8.0
pip: 21.3.1
conda: 4.13.0
pytest: None
mypy: None
IPython: 8.4.0
sphinx: None

@siranipour siranipour added bug needs triage Issue that has not been reviewed by xarray team member labels Dec 4, 2023
Copy link

welcome bot commented Dec 4, 2023

Thanks for opening your first issue here at xarray! Be sure to follow the issue template!
If you have an idea for a solution, we would really welcome a Pull Request with proposed changes.
See the Contributing Guide for more.
It may take us a while to respond here, but we really value your contribution. Contributors like you help make xarray better.
Thank you!

@max-sixty
Copy link
Collaborator

A few things going on here:

  • IIUC, we do want to raise an error on foo.groupby('bar').sum(). We're trying to be more explicit on the dimension argument, such that foo.groupby('bar').sum(...) is required to group over all dimensions.
    • But this seems to be inconsistent... Surprising .groupby behavior with float index #8263 is another case
    • IIUC the size of the groups shouldn't affect what raises an error
    • Possibly others have more insight here — are my claims on the intended behavior at least correct @pydata/xarray ?
  • The error message is quite bad — expected either '...' to reduce over all dimensions or one or more of ().. The of (). comes from there being no dimensions, it's not trying to suggest using ().

@max-sixty max-sixty added topic-groupby and removed needs triage Issue that has not been reviewed by xarray team member labels Dec 4, 2023
@dcherian
Copy link
Contributor

dcherian commented Dec 4, 2023

Sigh I'm responsible for that terrible error message. The real solution here is to use .groupby(..., squeeze=False)

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.

3 participants