Skip to content

Migrate DatetimeOrdinal to narwhals+numpy, add polars support - #1010

Merged
solegalli merged 7 commits into
narwhals-migrationfrom
narwhals-datetime-ordinal
Aug 30, 2026
Merged

Migrate DatetimeOrdinal to narwhals+numpy, add polars support#1010
solegalli merged 7 commits into
narwhals-migrationfrom
narwhals-datetime-ordinal

Conversation

@solegalli

Copy link
Copy Markdown
Collaborator

Replaces the pandas-only row-by-row implementation (pd.to_datetime + .apply(lambda x: x.toordinal())) with a vectorized one: string/categorical variables are parsed to a real Date/Datetime dtype via narwhals' str.to_datetime() (shared across backends), then the ordinal itself is computed as (days-since-epoch + epoch_ordinal), verified to match datetime.date.toordinal() exactly, including pre-epoch and year-1 dates.

Benchmarked the ordinal math at 10k/50k/100k rows x 1/2/10 columns:

  • old apply()-based pandas path vs a narwhals-generic dt.timestamp() path: 27x-234x faster, growing with row count (the old code was O(rows) in Python, this is fully vectorized).
  • narwhals dt.timestamp() vs a numpy datetime64[D] fast path on pandas: numpy wins by 3.4x-12x (bigger at low row counts, where per-call narwhals/polars-engine overhead dominates). This is a real, not minimal, gain, so pandas gets its own numpy branch (_transform_pandas: to_numpy().astype("datetime64[D]").astype("int64")), while polars stays on the narwhals dt.timestamp() path (_transform_narwhals), which was already fast enough (0.09-1.3ms) that a numpy round-trip through Arrow wouldn't pay for itself.

start_date parsing in init no longer imports pandas (pd.to_datetime -> dateutil.parser.parse, already a core dependency and already used elsewhere in feature_engine/variable_handling); datetime.date/datetime objects use their own .toordinal() directly, both stdlib.

Missing-value representation is now backend-native instead of forcing object-dtype + pd.NA: NaN/float64 for pandas, null/Int64 for polars - tests and docs normalize/document this instead of asserting one fixed dtype.

Bug found (pre-existing, not from this migration - verified against narwhals-migration base with git stash): the two "days from start_date" numbers in docs/user_guide/datetime/DatetimeOrdinal.rst were stale (-4343 and 3956 vs the actual -4342 and 3957); fixed against verified output. Also documents a real narwhals/polars limitation found while writing the polars doc example: polars' str.to_datetime() (unlike pandas' dateutil-backed pd.to_datetime) can't guess ambiguous or loosely-formatted date strings ("May-1989", "06/21/2012") without an explicit format - the polars example uses ISO-8601 strings instead, with a note explaining the difference.

Also found and fixed a latent bug this migration's own cross-backend tests exposed in the already-migrated shared _check_contains_na (feature_engine/dataframe_checks.py): nw.col([]) raises on the polars backend, which crashed fit() for return_empty=True + missing_values= "raise" + polars input (no variables found). Worked around locally by skipping the na-check when variables_ is empty (nothing to check anyway); flagged the shared function itself for a proper fix since other transformers hitting the same combination will have the same problem (spawned as a separate follow-up task).

Tests rewritten as one cross-backend parametrized test per behavior (@pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame])), 32 passed. Full tests/test_datetime suite: 152 passed, 2 pre-existing failures in test_datetime_features.py (DatetimeFeatures, unmigrated, unrelated file) confirmed present on narwhals-migration base too. flake8 and mypy clean. Module verified to import and run end-to-end on polars with pandas import blocked. sphinx -W build has the same single pre-existing linkcode_resolve warning as the unmigrated base, nothing new.

Replaces the pandas-only row-by-row implementation (pd.to_datetime +
.apply(lambda x: x.toordinal())) with a vectorized one: string/categorical
variables are parsed to a real Date/Datetime dtype via narwhals'
str.to_datetime() (shared across backends), then the ordinal itself is
computed as (days-since-epoch + epoch_ordinal), verified to match
datetime.date.toordinal() exactly, including pre-epoch and year-1 dates.

Benchmarked the ordinal math at 10k/50k/100k rows x 1/2/10 columns:
- old apply()-based pandas path vs a narwhals-generic dt.timestamp()
  path: 27x-234x faster, growing with row count (the old code was O(rows)
  in Python, this is fully vectorized).
- narwhals dt.timestamp() vs a numpy datetime64[D] fast path on pandas:
  numpy wins by 3.4x-12x (bigger at low row counts, where per-call
  narwhals/polars-engine overhead dominates). This is a real, not
  minimal, gain, so pandas gets its own numpy branch
  (_transform_pandas: to_numpy().astype("datetime64[D]").astype("int64")),
  while polars stays on the narwhals dt.timestamp() path
  (_transform_narwhals), which was already fast enough (0.09-1.3ms) that
  a numpy round-trip through Arrow wouldn't pay for itself.

start_date parsing in __init__ no longer imports pandas (pd.to_datetime
-> dateutil.parser.parse, already a core dependency and already used
elsewhere in feature_engine/variable_handling); datetime.date/datetime
objects use their own .toordinal() directly, both stdlib.

Missing-value representation is now backend-native instead of forcing
object-dtype + pd.NA: NaN/float64 for pandas, null/Int64 for polars -
tests and docs normalize/document this instead of asserting one fixed
dtype.

Bug found (pre-existing, not from this migration - verified against
narwhals-migration base with git stash): the two "days from start_date"
numbers in docs/user_guide/datetime/DatetimeOrdinal.rst were stale
(-4343 and 3956 vs the actual -4342 and 3957); fixed against verified
output. Also documents a real narwhals/polars limitation found while
writing the polars doc example: polars' str.to_datetime() (unlike
pandas' dateutil-backed pd.to_datetime) can't guess ambiguous or
loosely-formatted date strings ("May-1989", "06/21/2012") without an
explicit format - the polars example uses ISO-8601 strings instead, with
a note explaining the difference.

Also found and fixed a latent bug this migration's own cross-backend
tests exposed in the *already-migrated* shared `_check_contains_na`
(feature_engine/dataframe_checks.py): nw.col([]) raises on the polars
backend, which crashed fit() for return_empty=True + missing_values=
"raise" + polars input (no variables found). Worked around locally by
skipping the na-check when variables_ is empty (nothing to check
anyway); flagged the shared function itself for a proper fix since other
transformers hitting the same combination will have the same problem
(spawned as a separate follow-up task).

Tests rewritten as one cross-backend parametrized test per behavior
(`@pytest.mark.parametrize("make_df", [pd.DataFrame, pl.DataFrame])`),
32 passed. Full tests/test_datetime suite: 152 passed, 2 pre-existing
failures in test_datetime_features.py (DatetimeFeatures, unmigrated,
unrelated file) confirmed present on narwhals-migration base too.
flake8 and mypy clean. Module verified to import and run end-to-end on
polars with pandas import blocked. sphinx -W build has the same single
pre-existing linkcode_resolve warning as the unmigrated base, nothing
new.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@solegalli
solegalli force-pushed the narwhals-migration branch 2 times, most recently from e9f7d69 to 04c88dc Compare August 30, 2026 15:49
solegalli and others added 5 commits August 30, 2026 20:01
- __init__ stores raw self.start_date (user param) instead of deriving
  self.start_date_ at construction; start_date is now parsed into
  self.start_date_ordinal_ in fit(). Restores get_params()/clone().
- Inline nwd.is_pandas_dataframe(X) in the if statements.
- Remove the "reorder variables to match train set" step in transform();
  columns are selected by name, so it wasn't needed.
- transform() now converts to narwhals once and back to native once in
  the per-backend helper, with no round-trips in between.
- Tests updated: invalid start_date now raises from fit(); stale
  known-bug comment in test_return_empty corrected.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- start_date param: document that datetime.date is also accepted.
- fit() docstring: note it parses start_date and can raise ValueError
  (the raise moved here from __init__).
- Doctests: `_ = dtf.fit(X)` since repr(dtf) now works and would
  otherwise echo in the >>> fit(X) line.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Comment thread feature_engine/datetime/datetime_ordinal.py Outdated
@solegalli
solegalli merged commit a98990a into narwhals-migration Aug 30, 2026
4 of 10 checks passed
@solegalli
solegalli deleted the narwhals-datetime-ordinal branch August 30, 2026 19:23
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