Skip to content

Let a group of nothing but nans take the fill value - #100

Merged
ml31415 merged 1 commit into
masterfrom
purepy-all-nan-group
Oct 8, 2026
Merged

ml31415 merged 1 commit into
masterfrom
purepy-all-nan-group

Conversation

@ml31415

@ml31415 ml31415 commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

The nan forms of the pure python implementation filter the nan values out of the input before grouping. When every value of the input is a nan, nothing is left to filter down to and unpacking the empty remainder raises

ValueError: not enough values to unpack (expected 2, got 0)

A group that loses all of its values takes the fill_value - the same a group missing from group_idx does - and that is what the numpy implementations already make of such a group (0 for nansum, 1 for nanprod, 0 for nanlen, False for nanall, -1 for nanargmin, nan for the rest). Now the same here.

The new test compares that group against the numpy implementation over all fifteen nan forms, for three inputs: the whole input a nan, a single all-nan group next to one that has a value left, and the same with complex values - the filter compares val == val since #98 landed, so a nan in either part has to empty the group as well. Without the change it fails with the ValueError above; the mixed input alone does not trigger it, which is why the first and the third case are in there.

Two notes:

  • The test calls aggregate_purepy directly. The parametrized tests do not reach the pure python nan forms: _is_implemented() reports them as not implemented, and that is more than a stale label - removing it makes 27 tests fail (12 test_var_with_nan_fill_value, 7 test_cmp, 2 test_default_fill_value, 5 complex). Untangling pure python's nan semantics is a project of its own and not part of this.
  • Rebased onto Support complex input in the aggregation functions #98. It rewrote this very line to filter with val == val instead of math.isnan, which is what makes the complex case above possible; the two changes resolved into one block.

Measured: 2395 passed, 174 skipped, 924 deselected - one more than master, the new test.

The nan forms of the pure python implementation filter the nan values out
before grouping.  When every value of the input is a nan, nothing is left to
filter down to, and unpacking the empty remainder raised

    ValueError: not enough values to unpack (expected 2, got 0)

A group that loses all of its values takes the fill value, the same a group
missing from group_idx does, and that is what the numpy implementations
already make of such a group: 0 for nansum, 1 for nanprod, 0 for nanlen,
False for nanall, -1 for nanargmin, nan for the rest.  It is the same now.

The new test compares that group against the numpy implementation over all
fifteen nan forms, for three inputs: the whole input a nan, a single all-nan
group next to one with a value, and the same with complex values - the
filter compares `val == val` since the complex support landed, so a nan in
either part has to empty the group as well.  Without the change it fails
with the ValueError above, which the second input alone does not trigger.

The implementation is called directly: the parametrized tests do not reach
the pure python nan forms, and that label is not merely stale - dropping it
makes 27 tests fail (12 test_var_with_nan_fill_value, 7 test_cmp, 2
test_default_fill_value, 5 complex, 1 test_complex_default_fill_value).
Untangling the nan semantics of this implementation is a project of its own.
@ml31415
ml31415 force-pushed the purepy-all-nan-group branch from 29ecaa0 to de42267 Compare October 8, 2026 14:40
@ml31415
ml31415 merged commit 60151ff into master Oct 8, 2026
10 checks passed
@ml31415
ml31415 deleted the purepy-all-nan-group branch October 14, 2026 11:10
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 this pull request may close these issues.

1 participant