Skip NaN in quantile() and median() - #310
Open
vahid-ahmadi wants to merge 1 commit into
Open
Conversation
NaN sorts to the end of np.argsort and its weight still counted toward
the cumulative distribution, so the inverse-CDF cutoff was pushed
upward:
MicroSeries([1.0, nan, 3.0], weights=[1, 1, 1]).median() # 3.0
MicroSeries([1.0, 3.0], weights=[1, 1]).median() # 1.0
Dropping a NaN row should give the same answer as never having had it.
Add skipna (default True), dropping NaN rows alongside the existing
zero-weight filter; skipna=False returns NaN when any value is NaN,
matching mean/var/std.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #302
Problem
quantilehad no NaN handling. NaN sorts to the end ofnp.argsort, and its weight still counted toward the cumulative distribution, so the inverse-CDF cutoff was pushed upward:Dropping a NaN row should give the same answer as never having had it. Instead the NaN row's weight made up a third of the distribution, moving the 0.5 cutoff past the second value onto the largest one.
To be precise about the expected value: this method is documented as inverse CDF (
survey::svyquantile), so the correct answer is1.0— an observed value — not pandas' interpolated2.0. This PR does not change the interpolation semantics, only the NaN handling.Change
skipna: bool = Trueonquantileandmedian.skipna=True, NaN rows are dropped in the same mask that already drops zero-weight rows (Skip zero-weight rows in MicroSeries.quantile #287), so their weight no longer enterscumsum.skipna=False, NaN is returned when any value is NaN — matchingmean(Fix mean() method to properly handle skipna parameter #269),var/std(Weighted std/var; warn on unweighted cov/corr fallthrough #290).Verification
main,test_quantile_skips_nanandtest_quantile_skipna_false_propagates_nanfail; all three pass with the change.MicroSeries([10, 20, 30], weights=[0, 1, 1]).quantile(0)->20.pd.isnahandles them, nonp.isnancast error).make test— 65 passed on pandas 3.0.5 and pandas 2.3.3.Marked as a
minorbump in the changelog fragment: defaultmedian/quantileresults change for data containing NaN.Note: the lint job will be red until #307 merges (unrelated ruff 0.16 issue on
main).