Migrate DatetimeSubtraction to narwhals+numpy, add polars support - #1012
Open
solegalli wants to merge 1 commit into
Open
Migrate DatetimeSubtraction to narwhals+numpy, add polars support#1012solegalli wants to merge 1 commit into
solegalli wants to merge 1 commit into
Conversation
Ports DatetimeSubtraction (extends the already-migrated BaseCreation) to narwhals, adding native polars support and removing the pandas-only computation path, following the RelativeFeatures precedent. Benchmarked pandas-native vs narwhals+numpy on pandas vs narwhals+numpy on polars at 10k/50k/100k rows x 1/2/10 datetime-pair combinations. Extracting each unique variable to a numpy datetime64 array once, then subtracting and dividing with plain numpy ops, is a clear MERGE win - no is_pandas branch needed for the arithmetic itself: rows=100000 pairs=10 | pandas_native=5.934ms | narwhals+numpy(pandas)= 3.011ms (0.51x) | narwhals+numpy(polars)=1.282ms (0.22x) End-to-end (including datetime parsing), the new pandas path is also consistently faster than the old pandas-only implementation (0.55x-0.96x across the grid), and polars is 4-20x faster than pandas at scale once parsing cost is amortized over more rows. "Y"/"M" output units are non-linear numpy timedelta units, so both the diff and the unit divisor are cast to timedelta64[ns] before dividing (numpy can't otherwise find a common divisor) - this mirrors what pandas does internally for Timedelta / Timedelta and was verified against all 14 supported output_unit values. Datetime parsing (dayfirst/yearfirst/utc/format) is inherently backend-specific, so it keeps a real is_pandas branch: the pandas path calls pandas.to_datetime via nw.get_native_namespace() (no "import pandas") to preserve exact prior behaviour; the non-pandas path uses narwhals' str.to_datetime first, then falls back to per-value dateutil parsing (honouring dayfirst/yearfirst/utc) for ambiguous formats narwhals can't infer - the same flexible, cross-backend date guessing check_datetime_variables/find_datetime_variables already promise, so a column that passes fit() can always be parsed in transform() on any backend. No bugs found in DatetimeSubtraction itself. Two pre-existing failures in test_datetime_features.py (DatetimeFeatures index/NaN handling) and 68 repo-wide pre-existing failures elsewhere are unchanged before/after this change (confirmed via git stash) and belong to other, not-yet-migrated modules. Rewrote tests/test_datetime/test_datetime_subtraction.py to parametrize every dataframe-dependent test over pandas and polars via make_df (122 tests, up from 83), and added a "With polars" section to DatetimeSubtraction.rst, verifying every doc example (old and new) against actual output. Co-Authored-By: Claude Sonnet 5 <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.
Ports DatetimeSubtraction (extends the already-migrated BaseCreation) to narwhals, adding native polars support and removing the pandas-only computation path, following the RelativeFeatures precedent.
Benchmarked pandas-native vs narwhals+numpy on pandas vs narwhals+numpy on polars at 10k/50k/100k rows x 1/2/10 datetime-pair combinations. Extracting each unique variable to a numpy datetime64 array once, then subtracting and dividing with plain numpy ops, is a clear MERGE win - no is_pandas branch needed for the arithmetic itself:
rows=100000 pairs=10 | pandas_native=5.934ms | narwhals+numpy(pandas)=
3.011ms (0.51x) | narwhals+numpy(polars)=1.282ms (0.22x)
End-to-end (including datetime parsing), the new pandas path is also consistently faster than the old pandas-only implementation (0.55x-0.96x across the grid), and polars is 4-20x faster than pandas at scale once parsing cost is amortized over more rows. "Y"/"M" output units are non-linear numpy timedelta units, so both the diff and the unit divisor are cast to timedelta64[ns] before dividing (numpy can't otherwise find a common divisor) - this mirrors what pandas does internally for Timedelta / Timedelta and was verified against all 14 supported output_unit values.
Datetime parsing (dayfirst/yearfirst/utc/format) is inherently backend-specific, so it keeps a real is_pandas branch: the pandas path calls pandas.to_datetime via nw.get_native_namespace() (no "import pandas") to preserve exact prior behaviour; the non-pandas path uses narwhals' str.to_datetime first, then falls back to per-value dateutil parsing (honouring dayfirst/yearfirst/utc) for ambiguous formats narwhals can't infer - the same flexible, cross-backend date guessing check_datetime_variables/find_datetime_variables already promise, so a column that passes fit() can always be parsed in transform() on any backend.
No bugs found in DatetimeSubtraction itself. Two pre-existing failures in test_datetime_features.py (DatetimeFeatures index/NaN handling) and 68 repo-wide pre-existing failures elsewhere are unchanged before/after this change (confirmed via git stash) and belong to other, not-yet-migrated modules.
Rewrote tests/test_datetime/test_datetime_subtraction.py to parametrize every dataframe-dependent test over pandas and polars via make_df (122 tests, up from 83), and added a "With polars" section to DatetimeSubtraction.rst, verifying every doc example (old and new) against actual output.