Repository navigation
Let a group of nothing but nans take the fill value - #100
Merged
Merged
Conversation
ml31415
force-pushed
the
purepy-all-nan-group
branch
from
October 8, 2026 14:25
5680517 to
29ecaa0
Compare
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
force-pushed
the
purepy-all-nan-group
branch
from
October 8, 2026 14:40
29ecaa0 to
de42267
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
nanforms 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 raisesA group that loses all of its values takes the
fill_value- the same a group missing fromgroup_idxdoes - and that is what the numpy implementations already make of such a group (0fornansum,1fornanprod,0fornanlen,Falsefornanall,-1fornanargmin,nanfor the rest). Now the same here.The new test compares that group against the numpy implementation over all fifteen
nanforms, 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 comparesval == valsince #98 landed, so a nan in either part has to empty the group as well. Without the change it fails with theValueErrorabove; the mixed input alone does not trigger it, which is why the first and the third case are in there.Two notes:
aggregate_purepydirectly. The parametrized tests do not reach the pure pythonnanforms:_is_implemented()reports them as not implemented, and that is more than a stale label - removing it makes 27 tests fail (12test_var_with_nan_fill_value, 7test_cmp, 2test_default_fill_value, 5 complex). Untangling pure python's nan semantics is a project of its own and not part of this.val == valinstead ofmath.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.