Skip to content

Support TimeDelta columns in Median and Percentiles - #824

Open
ChrisJr404 wants to merge 1 commit into
wireservice:masterfrom
ChrisJr404:median-timedelta
Open

Support TimeDelta columns in Median and Percentiles#824
ChrisJr404 wants to merge 1 commit into
wireservice:masterfrom
ChrisJr404:median-timedelta

Conversation

@ChrisJr404

Copy link
Copy Markdown

This wraps up the Median half of #761. Sum and Mean already accept TimeDelta columns, so it was a bit surprising that Median still raised a DataTypeError on them, especially since timedeltas sort and average just fine.

The only thing in the way was the type check. Percentiles does the actual work for Median, and its arithmetic (averaging the two adjacent values and dividing by two) already works on timedeltas, so I loosened the check in both Percentiles and Median to allow TimeDelta alongside Number, and made Median.get_aggregate_data_type return the column's own data type the way Sum and Mean do. The rest of the percentile family (Quartiles, Quintiles, Deciles, IQR, MAD) validates for Number on its own, so nothing else changes.

Added tests for median and percentiles over a timedelta column plus an all-nulls case, and a changelog line.

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