Repository navigation
Preserve missing datetimes for median imputation - #1650
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues block approval of the datetime missing-value imputation changes.
No open issues 🎉
What changed in this PR
This PR restores median imputation for missing datetime features in FLAML’s tabular preprocessing, while preserving valid-date conversion and existing calendar features.
Changes:
- Preserve missing timestamps as
np.nanduring both training and prediction ordinal conversion. - Add 11 test cases covering naive and UTC timestamps, missing-value scenarios, unchanged valid-date behavior, and regression/classification preprocessing integration.
| File | Description |
|---|---|
test/automl/test_datetime_missing.py |
Adds datetime imputation regression tests and preprocessing integration coverage. |
flaml/automl/data.py |
Guards both ordinal conversions so missing timestamps reach the numeric imputer. |
Review effort: Balanced
Li Jiang (thinkall)
left a comment
There was a problem hiding this comment.
Approved after a clean whole-PR review of exact head 23046b00e6c21a47ca2eb96cdbe45724f390146b.
The matching fit/transform guards preserve missing timestamps for the existing training-fitted median imputer while leaving valid ordinals, generated calendar features and existing column-removal behavior intact. review-code Low execution with actual gpt-6.1-sol found no issues, and the complete source/test changes were also checked against the regression behavior.
Independent native validation passed the same 28 focused cases on each of two existing Windows stacks: Python 3.10.19/NumPy 1.26.4/pandas 2.3.3 and Python 3.13.5/NumPy 2.3.3/pandas 3.0.5. These are 56 executions of 28 distinct cases: 11 new datetime cases and 17 unchanged categorical neighbors. Against pristine main c0aceba9, the identical tests produce eight intended missing-ordinal failures and twenty passes on each stack; current fixes all eight, including regression/classification task-validation and preprocessing integration.
This validates native CPU preprocessing, not estimator training, GPU/Spark/distributed execution, the full optional/minimum-version matrix or performance. Author-reported pandas 3.1.0 is not being presented as driver-executed validation.
Immediately before approval, the same non-draft/non-self head was checked for resolved review threads, an explicit current-head Balanced Copilot Approval recommended verdict, successful applicable check contexts, and successfully completed actual Build, docs and Code formatting workflow runs. The skipped release job is explicitly inapplicable to pull requests, not counted as passed validation.
Posted by thinkall-agent-auto-reviewer
## Summary Merge public main through 386ff40 into the existing consolidation branch. Resolve five conflicts while preserving upstream seasonal forecasts, training-log append behavior, datetime preprocessing, portfolio isolation, search-space fixes, and the public dependency policy. Address introduced and tightly coupled review findings: use the fitted calendar for integer forecasts, reject malformed prediction grids before backend calls, measure forecast validation transforms without charging training history, retain nullable and wide time indexes with bounded-memory gcd calculation, revalidate Spark casts on the full column, and keep public SearchState AutoFE disabled by default. Strengthen append/non-resume, missing-class, lag-bound, exported-input, and conversion regression coverage. ## Prompting Intent The engineer asked to use review-code on PR #1545, make needed changes, push the existing upstream branch, and make the code ready for merging. Preserve ordinary public installs/defaults, enable Fabric behavior explicitly or through local runtime detection, and do not merge or bypass human review approval. ## Linked Sources - Current PR and discussion: #1545 - Public main: 386ff40 - Seasonal forecast definitions: #1611 - Safe training-log append: #1615 - Panel time-index steps: #1636 - Missing datetime preprocessing: #1650 - Pandas preprocessing compatibility: #1648 - Fresh review decision and dispositions: reviews/pr-1545/task-1545-attempt-2-stage-attempt-1-review-1-grok-4.7.md - All six fresh lenses and two focused verifications: reviews/pr-1545/task-1545-attempt-2-*.md - Runtime/API contracts: flaml/fabric/README.md ## Rationale The PR head was unchanged and previously CI-green, but current main introduced 16 focused commits and new conflicts. A fresh Medium follow-up used the current catalog's complementary grok-4.7 reviewer at xhigh reasoning for six concurrent lenses, then independent correctness and coverage verification. Share partition transformation logic so forecast timing reflects held-out work without changing transformed output or training-only operations. Preserve public calendar and lag semantics rather than retaining obsolete PR-only expectations. Keep threshold-based coercion explicit, but prevent a biased Spark sample from choosing a cast unsupported by the full column. All eight skill-owned reports ship with the changes and retain their original findings and verdicts. Current-base AST comparisons and dependency probes distinguish inherited concerns and unsubstantiated claims from merge regressions; no clean reviewer sign-off or unrelated lifecycle/sampling redesign is claimed. This is maintenance of an existing GitHub PR with no supplied ADO TaskId; its existing infrastructure labels and GitHub sources provide the work linkage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b0e4942d-f489-4b42-8a90-097ede4e88f7
Why are these changes needed?
A missing datetime feature becomes ordinal
1whenDataTransformermapsdatetime.toordinaloverNaT. The numeric median imputer then treats it as a valid date instead of a missing value. For example, a missing timestamp among dates around 2020-02-29 becomes1rather than the learned median737484; the prediction path has the same problem, including batches containing only missing timestamps.Guard both ordinal conversions so missing timestamps remain
np.nanfor the existing training-fitted median imputer. Valid datetime conversion, generated calendar features and the existing all-missing training-column removal are preserved.Eleven new cases cover naive/UTC timestamps, missing training values, prediction-only and entirely missing prediction batches, valid-date/input-preservation controls, and native
GenericTask.validate_data/AutoML.preprocessintegration for regression and classification. No estimator is fitted by these tests.On Windows/Python 3.12.14, the identical final 28-case suite (11 new cases and 17 unchanged categorical preprocessing neighbors) gives 8 failed, 20 passed on pristine base
c0aceba952a77c28f2cf921f9b1732214bfb7bfaand 28 passed after the fix, separately on actual pandas 2.3.3 and 3.1.0. All failures are the missing ordinal1versus the learned date median. Fullpre-commit run --all-files(15 configured hooks) andgit diff --checkpass.Existing datetime-accessor/drop deprecations on pandas 3.1 and a categorical downcast warning on pandas 2.3 remain visible. The separate deprecation work in #1648 is unchanged. This validates CPU preprocessing correctness; no estimator/model training, GPU/Spark/distributed, full optional-dependency suite, minimum-version matrix or performance claim is made.
Related issue number
Independently reproduced; no linked issue.
Checks
Implementation and validation were performed with AI assistance.