BUG: fix Table.group_by().groups.aggregate() for multidimensional columns - #20410
bhuvan-somisetty wants to merge 1 commit into
Conversation
…umns ColumnGroups.aggregate() computed per-group means for multidimensional columns as np.add.reduceat(par_col, i0s) / np.diff(self.indices), where the count array only accounts for the leading (row) axis. Dividing a multidimensional reduceat result by this 1-D count array either broadcasts silently against the wrong axis (giving wrong numbers with no error) whenever the number of groups happens to equal a trailing dimension size, or raises a broadcasting error that gets caught and turned into a warning, silently dropping the column from the aggregated table. Reshape the per-group counts to broadcast only against the row axis, matching the fix already applied to aggregate_downsample() for the same underlying issue (astropy#13225). Fixes astropy#20409
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
This comment was marked as resolved.
This comment was marked as resolved.
|
CircleCI, docs build, and pre-commit are all passing. Looks like the GH Actions workflows are stuck waiting on approval since this is my first PR here - could a maintainer approve them so the rest of CI can run? Thanks! |
|
Good catch, sorry for missing that - added an AI usage disclosure section to the PR description. Let me know if you'd like more detail. |
This comment was marked as resolved.
This comment was marked as resolved.
|
Filled out the AI Disclosure section in the PR description - I used Claude Code (Claude Sonnet 5) to help investigate, write the fix/tests, and draft the PR/issue text, and I've reviewed and tested everything before it was posted. Checked the certification box too. Thanks for flagging it. |
Description
ColumnGroups.aggregate()(used byTable.group_by(...).groups.aggregate()) has a fast path fornp.meanthat does:np.diff(self.indices)is a 1-D array of per-group row counts. For a multidimensional column,np.add.reduceat(par_col, i0s)has shape(n_groups, *trailing_dims), and dividing it by the 1-D counts array broadcasts against the last axis instead of the row axis. This means:n_groupshappens to equal a trailing dimension size, the division silently succeeds but produces wrong numbers, with no error or warning at all;This fixes it by reshaping the per-group counts so the division only broadcasts against the row axis, preserving the trailing dimensions. It's the same underlying issue as #13225, which was already fixed for
TimeSeries.aggregate_downsample()— this applies the same fix to theTable.group_by().groups.aggregate()code path.Added a regression test covering both failure modes described above (wrong values when shapes coincide, dropped column otherwise), and a changelog fragment.
Fixes #20409
AI Disclosure
If AI tools were used to develop this pull request, describe the tools including specific model and version, how they were used, and what content is AI generated. Otherwise enter "N/A".
I used Claude Code (Claude Sonnet 5) as an assistant throughout this contribution:
astropy/table/groups.py.Merge method