Implement Series.quantile #517
Merged
Conversation
This file contains 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
crusaderky
reviewed
Dec 11, 2023
| @@ -1285,6 +1286,13 @@ def rename(self, index): | |||
| return new_collection(expr.RenameSeries(self.expr, index)) | |||
| raise NotImplementedError(f"passing index={type(index)} is not supported") | |||
|
|
|||
| def quantile(self, q=0.5, method="default"): | |||
There was a problem hiding this comment.
Out of scope: this could get easily confused with the method parameter of pandas.DataFrame.quantile.
We should rename it both here and in dask/dask to dask_method or something.
There was a problem hiding this comment.
Yep good point, but I’d rather do this not with all the other changes
Co-authored-by: crusaderky <[email protected]>
crusaderky
reviewed
Dec 11, 2023
Comment on lines
+1672
to
+1673
| with pytest.raises(AssertionError): | ||
| df.x.quantile(q=[]).compute() |
There was a problem hiding this comment.
pandas returns an empty series here. Doubt we care to diverge on such an edge case though.
There was a problem hiding this comment.
Yeah I thought this was kind of pointless, but I might put up a follow up anyway
crusaderky
approved these changes
Dec 11, 2023
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.
We can't really test tdigest at the moment, it's not compatible with the newest NumPy and I don't want to create a new ci job for this.
Tested it locally and it works for now