You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(automl): treat a DatetimeIndex as the ts_forecast time column - #1614
ts_forecast expects the timestamp to be a column (time_col, which defaults to the first column). When the timestamps are passed as the pandas DataFrameindex instead — the idiomatic shape, and what the reporters of #1229 and of the audit in #1570 (item A1) ran into — TimeSeriesTask.validate_data inferred time_col from dataframe.columns[0], i.e. a value column that is frequently the label itself, and the real timestamps were never read.
What that looks like on main, with pandas>=2.0, numpy:
pd.to_datetime(df['y']) reads numeric label values as nanoseconds since epoch (1970-01-01), collapsing rows and throwing misleading errors (AssertionError: Duplicate timestamp values or ValueError: pandas dtypes must be int, float or bool).
Solution & Scope of Changes
Consistent DatetimeIndex Promotion:
When time_col is inferred and dataframe.index is a DatetimeIndex while the candidate column is not datetime-like, promote the index to a column before timestamp conversion (_promote_datetime_index_if_needed).
Factored promotion across the entire lifecycle: training data, validation holdout data (X_val), and prediction inputs (X).
Explicit index names (including empty string "" and integer 0) are preserved (df.index.name is None fallback to "ds"), ensuring prediction-form equivalence between raw-index frames and reset_index() frames.
Guaranteed unique column names: avoids collisions with existing feature columns or target names (e.g., ds_1, ds_2).
Target Alignment & Compatibility:
Preserved original positional contract for conventional pd.Series and RangeIndex targets.
When both feature and target have a DatetimeIndex, alignment is validated and paired by label (reindex), raising ValueError on label mismatches to prevent silent data corruption.
Precedence: user-supplied time_col is never overridden; existing valid date/timestamp columns (including convertible date strings) are preserved.
Restrict named-column presence checks to pd.DataFrame so supported NumPy feature arrays reach normalization without premature rejection.
Restores user input DatetimeIndex on prediction outputs across all estimators, including estimators that reset output index (e.g. Orbit, Prophet).
Comprehensive test suite in test/automl/test_ts_forecast_datetime_index.py with 41 dedicated tests covering all input permutations, edge cases, and runtime behaviors:
All 15 GitHub Actions workflow jobs completed with SUCCESS:
build (ubuntu-latest, 3.10 - 3.13): Passed
build (windows-latest, 3.10 - 3.13): Passed
build (windows-11-arm, 3.11 - 3.13): Passed
pre-commit-check: Passed
checks: Passed
license/cla: Passed
Checks
I've used pre-commit to lint the changes in this PR.
I've included any doc changes needed for https://microsoft.github.io/FLAML/. (No public API change: time_col remains optional and behavior is backwards-compatible).
I've added tests corresponding to all changes introduced in this PR.
ts_forecast expects the timestamp in a column, so validate_data infers
`time_col = dataframe.columns[0]` when the caller does not pass one. For a
frame whose timestamps are the DataFrame index -- the idiomatic pandas shape
-- that inference picks a value column, usually the label itself, and the
conversion at `pd.to_datetime()` then reads those numbers as nanoseconds since
the epoch instead of failing. The 120 real timestamps collapse onto
1970-01-01, `remove_ts_duplicates` silently drops the rows that collide, and
the run dies with an unrelated-looking error:
AssertionError: Duplicate timestamp values with different values for other columns.
With a wider numeric range nothing collides and the same input instead fails
later with "pandas dtypes must be int, float or bool" on the lag features.
Neither message mentions the index.
When `time_col` was inferred (never user-supplied), the index is a
DatetimeIndex, and the inferred column is not datetime-like, promote the index
to a column before the conversion. It keeps the index's own name, falling back
to `ds`, and the decision is logged. A supplied `time_col`, a first column that
is already datetime-like, and frames with no timestamps at all all keep their
current behaviour; the existing error text is deliberately unchanged.
Part of microsoft#1570 (Track A, item A1).
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the complete PR: changes are required.
flaml/automl/task/time_series_task.py:165: index-name collisions can make reset_index() fail or select the wrong timestamp column when existing columns are named like the index, index, or ds. Materialize the index under a guaranteed-unique name and add collision tests.
flaml/automl/task/time_series_task.py:156,180,405: DatetimeIndex promotion is applied only to training data. Indexed validation data and indexed future prediction inputs still fail. Factor this into shared time-series input normalization and apply it consistently while preserving explicit time_col precedence.
Guaranteed-unique column name resolution to prevent collisions:
Factored DatetimeIndex promotion into _promote_datetime_index_if_needed(df, time_col=None, is_inferred=False).
If the index name (or index / ds) already exists in df.columns, we iteratively find a collision-free target name (e.g. index_1, ds_1) before calling reset_index().
Added unit test test_index_name_collision_resolves_to_unique_column to verify that existing columns with the same name are preserved without conflict.
Consistent DatetimeIndex promotion across training, validation, and inference:
Reused _promote_datetime_index_if_needed across the full input lifecycle:
In validate_data() for both training dataframe and validation X_val.
In preprocess() for prediction inputs X passed with a DatetimeIndex and no explicit time_col.
Preserved explicit time_col precedence in all cases.
Added unit tests test_validation_data_with_datetime_index and test_predict_with_datetime_index to ensure parity across fit, validate, and predict paths.
All formatting checks and regression tests have passed. Please let us know if any further adjustments are needed!
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the current PR: changes are still required.
flaml/automl/task/time_series_task.py:174-180: promoting an indexed X_val resets its index to RangeIndex, but normalize_ts_data() only realigns y_val when it is a Series, not when it is a DataFrame. An indexed DataFrame target is concatenated on its original DatetimeIndex; a two-row validation set becomes four rows with missing timestamp/label values. Preserve aligned indexes or reset/validate the target index consistently, and test DataFrame y_val with a DatetimeIndex.
test/automl/test_ts_forecast_datetime_index.py:88-110: both new regression tests fail in the current Build workflow. The collision test accesses nonexistent AutoMLState.train_data; the validation test passes y in X_train and again in y_train, creating duplicate target columns and ValueError: y should be a 1d array, got an array of shape (120, 2). Correct the test fixtures/assertions and rerun the build matrix before merging.
Target index alignment for DataFrame y (normalize_ts_data & validate_data):
In normalize_ts_data() (flaml/automl/time_series/ts_data.py), added index realignment for pd.DataFrame targets (y_train_all.index = X_train_all.index), matching the behavior previously only applied to pd.Series and np.ndarray.
In validate_data() (flaml/automl/task/time_series_task.py), ensured y_val (and y_train_all when passed as X_train_all/y_train_all) is realigned to the promoted feature index whenever it is a Series or DataFrame.
Added dedicated test test_validation_data_with_dataframe_target_and_datetime_index to verify that a validation set with a DataFrame target and a DatetimeIndex maintains exact row alignment and produces 0 NaN/NaT values.
Corrected test fixtures and assertions:
In test_index_name_collision_resolves_to_unique_column: updated assertions to inspect automl._feature_names_in_ and automl.data_size_full rather than nonexistent _state.train_data.
In test_validation_data_with_datetime_index: fixed the test fixture so that y is not duplicated between X_train and y_train, and asserted eval_method == "holdout" and prediction length.
Added test_xtrain_ytrain_with_datetime_index to verify (X_train, y_train) input pairs with DatetimeIndex.
All 11 unit tests in test_ts_forecast_datetime_index.py now pass cleanly. Thank you again!
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the complete current PR: changes are still required.
flaml/automl/task/time_series_task.py:154-158,192-196 and flaml/automl/time_series/ts_data.py:549-555: the new alignment fix unconditionally overwrites DataFrame target indexes with the feature-frame index when they differ, even without promoting a DatetimeIndex. For X indexed in timestamp order and y containing the same timestamps in a different row order, pandas previously matched rows by timestamp; now y values are assigned positionally to the wrong observations without error. For example, X at Jan 1/2/3 and y indexed Jan 3/1/2 with values 300/100/200 previously gave aligned targets 100/200/300, but now gives 300/100/200. Align y by its original labels before index promotion, or explicitly validate/reject mismatched indexes instead of silently overwriting them; cover reordered DataFrame targets for both training and validation. The earlier collision/test failures appear addressed, but this new behavior is a data-integrity blocker.
We have completely resolved the target index alignment issue in commit 7d92b36d by strictly adhering to pandas label-based alignment semantics rather than positional index overwrites:
Introduced _align_y_to_X_and_promote in TimeSeriesTask (flaml/automl/task/time_series_task.py) and updated normalize_ts_data (flaml/automl/time_series/ts_data.py).
If feature index and target index do not match, y is aligned by label via y.reindex(X.index).
If reindex introduces unaligned missing labels (new NaNs), it explicitly raises ValueError: Target index labels do not match feature index labels. to prevent silent data corruption.
DatetimeIndex promotion only resets the target index to RangeIndex after verified label alignment has completed.
Added Comprehensive Regression Tests:
test_training_with_reordered_dataframe_target_aligns_by_label: verifies that shuffled timestamp labels in DataFrame y_train align to X_train by label.
test_validation_with_reordered_dataframe_target_aligns_by_label: verifies that shuffled timestamp labels in DataFrame y_val align to X_val by label during holdout evaluation.
test_mismatched_target_index_raises_value_error: verifies that mismatched target/feature indexes raise an explicit ValueError.
All 14 unit tests in test_ts_forecast_datetime_index.py and test_ts_data.py now pass cleanly without formatting issues. Thank you!
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the current PR: changes are still required.
flaml/automl/task/time_series_task.py:146-158,418-440 and flaml/automl/time_series/ts_data.py:549-562: the new alignment helper reindexes every pandas target by the feature frame's labels before index promotion. When X_train has a DatetimeIndex and y_train is a conventional pd.Series(values, name="y") with a default RangeIndex, reindexing produces all-NaN targets and raises ValueError: Target index labels do not match feature index labels. The pre-PR Series path explicitly assigned X_train's index to y, supporting this positional pairing. Keep that positional Series contract (or handle it explicitly before label-based alignment) while retaining label alignment for a reordered DatetimeIndex target. Add a DatetimeIndex-X/RangeIndex-Series regression for training and validation.
The previous DataFrame ordering bug and CI test failures appear resolved; the current build and formatting checks pass.
…tures
Preserve the positional Series contract when X has a DatetimeIndex and y has a RangeIndex (or non-DatetimeIndex Series), while retaining label alignment for targets with DatetimeIndex. Add regressions for training and validation.
We have updated _align_y_to_X_and_promote and normalize_ts_data to preserve the positional contract when X has a DatetimeIndex and y is a conventional pd.Series (or RangeIndex target), while strictly retaining label-based alignment when the target has a DatetimeIndex.
Specifically:
In _align_y_to_X_and_promote and normalize_ts_data:
If X has a DatetimeIndex and y has a default RangeIndex (or is a Series without DatetimeIndex), y.index = X.index maintains positional parity.
If y has a DatetimeIndex, y is reindexed by X.index to align by timestamp labels, and raises ValueError if mismatched labels introduce NaNs.
Added regressions in test/automl/test_ts_forecast_datetime_index.py:
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the current PR: changes are still required.
flaml/automl/task/time_series_task.py:422-451 and flaml/automl/time_series/ts_data.py:549-564: the new rule treats any datetime-indexed Series target as label-indexed against X.index. When X is the standard frame with a ds timestamp column and a positional RangeIndex, while y_train is a Series indexed by those same timestamps, reindex(X.index) produces all missing labels and now raises ValueError: Target index labels do not match feature index labels. Before this PR, the Series was paired positionally to the three feature rows and fit normally. This also affects ndarray X, which is converted to a RangeIndex DataFrame. Only perform datetime-label alignment when X itself has a matching DatetimeIndex; otherwise preserve the existing positional Series contract (or align against the actual timestamp column explicitly). Add training and validation regressions for RangeIndex X with a datetime column and DatetimeIndex y.
The prior DatetimeIndex-X/RangeIndex-y defect is addressed. Current formatting checks and completed build legs pass; the remaining build matrix is still running.
…imeIndex
Preserve positional Series contract when X has a RangeIndex (e.g. timestamps in a column or ndarray) and y has a DatetimeIndex. Only perform datetime-label alignment when both X and y have a DatetimeIndex. Add training and validation regressions for RangeIndex X with a datetime column and DatetimeIndex y.
We have updated _align_y_to_X_and_promote and normalize_ts_data so that datetime-label alignment is only performed when bothX and y have a DatetimeIndex.
Specifically:
When X has a RangeIndex (such as when timestamps are stored in a column like ds, or when X is an ndarray):
y (whether a Series with a DatetimeIndex or RangeIndex) preserves the original positional Series contract (y.index = X.index).
When both X and y have a DatetimeIndex:
y is reindexed by X.index to align by timestamp labels, raising ValueError if label mismatches introduce missing values.
Added regressions in test/automl/test_ts_forecast_datetime_index.py:
test_training_with_range_index_x_datetime_col_and_datetime_index_series_target: verifies positional pairing when X_train has a ds column and a RangeIndex while y_train has a DatetimeIndex.
test_validation_with_range_index_x_datetime_col_and_datetime_index_series_target: verifies positional pairing for validation sets when X_val has a ds column and a RangeIndex while y_val has a DatetimeIndex.
The reason will be displayed to describe this comment to others. Learn more.
Overall review of the complete current PR: approved.
DatetimeIndex timestamps are promoted under a unique column name for training, validation, and prediction, with explicit time columns retaining precedence. The latest fix restores positional pairing for Series targets when features have a RangeIndex and a timestamp column, while retaining label-based alignment when both feature and target indexes are DatetimeIndexes; regression tests cover both shapes for training and validation. Earlier index-collision, duplicate-target test, and mismatched-row issues are addressed. Formatting and completed CI build legs pass; some build legs were still running at review time. No blocking code issues found.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Estimators that reset prediction indexes still cause NaNs when results are assigned back to DatetimeIndex inputs.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Restore input index on final predictions to prevent NaNs
flaml/automl/task/time_series_task.py:629
This restores the feature index, not the final prediction index. Orbit.predict() explicitly resets its output index (ts_model.py:211–228), while Prophet returns the library-generated forecast['yhat'] without restoring the input index (ts_model.py:325–328). AutoML.predict() then returns the estimator result unchanged. For Orbit, assigning the predicted target column back to a DatetimeIndex input therefore produces all NaNs. Restore the input index on final pandas prediction results, accounting for any estimator row reordering, and add regression coverage for an estimator that resets its output index; the current lgbm-only coverage misses this case.
The reason will be displayed to describe this comment to others. Learn more.
Review at e4944d44ad6b92665596c343d346948fa5bcce2b: documentation clarification verified; this is not an approval.
The test-module introduction is now consistent with the existing index promotion, naming, alignment and NumPy-input coverage. Production diff is empty; executable test AST is identical after excluding the initial docstring. The completed Low documentation review is CLEAN (actual gpt-6.1-sol), with verified input/model/read provenance and full all-file pre-commit.
This is recovered completed evidence, not a second harness execution. The prior 40-test result is reused on executable-identical code, not claimed as a new test run. This descriptive change does not establish a new estimator-output-index contract or resolve the current Copilot request for final prediction-index coverage on Orbit/Prophet. Those optional backend paths were not executed here.
Approval remains withheld because an explicit current-head Balanced Copilot approval recommendation is absent. No new executable defect is attributed to this documentation-only delta.
Canonical evidence was committed at the exact inspected source parent after full all-file hooks passed. Historical b017 evidence is preserved separately and no stale review is posted for it. No contributor implementation was edited or pushed.
… that reset output index
- In AutoML.predict, preserve orig_index before preprocessing and restore it on returned Series/DataFrame predictions when lengths match (resolving Copilot review on prediction index restoration).
- In Orbit.predict and Prophet.predict, restore X.index on output predictions if lengths match.
- Add regression test test_predict_restores_datetime_index_for_estimator_that_resets_output_index covering estimators that reset their output index to RangeIndex, verifying that assigning predictions back to a DatetimeIndex input produces no NaNs.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed current d5fe77ca8afe39b944261e5182f0bd47f071b017 with review-code (Low, gpt-6.1-sol) and reassessed the complete PR after its corrective commits. No actionable source issue remains from the cases I independently reproduced.
The strict index-name regression is corrected without disabling equality checks, and the latest coverage includes a fitted native ARIMA backend. On this exact head, the full affected selection passed 51 tests (13 deselected), and the focused selection passed 10 tests. Genuine public AutoML/ARIMA training and shuffled in-sample prediction now preserve timestamp/value association; the supported NLP list/Series custom-learner path also returns the expected finite prediction. Earlier failing cases and original review outcomes are retained as historical evidence, not reclassified as originally clean.
The concrete ARIMA ordering counterexample associated with the existing thread passes native validation. I have not resolved another reviewer's thread or treated that validation as an explicit Copilot approval recommendation. Optional Prophet/Orbit/TFT/SARIMAX training and additional local platform matrices were not executed; their boundary controls are not claimed as a full training matrix.
Approval is withheld: current-head Balanced Copilot does not explicitly recommend approval; 2 unresolved review thread(s); applicable check contexts are not all successful; actual current-head pull_request workflows have not all completed successfully.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 73cb38b36fe2baa4207633ac8e6561ef9cdd3bb8 with review-code (Low, gpt-6.1-sol): no actionable source finding in the inspected scope.
The timestamp conversion now runs after normalization, so one- and two-dimensional NumPy ISO-string inputs follow the same conversion as DataFrames. Separate exact prior/current native public AutoML runs reproduce the prior missing-calendar-feature KeyErrors and verify the fix: both current array forms return 12 finite predictions matching equivalent datetime DataFrames on the same fitted models, without mutating caller arrays.
The affected current selection passed 52 tests (13 deselected). This confirms the concrete array conversion case, not an unexecuted full optional-backend/platform matrix. No other reviewer's shared-ordering thread was resolved by this driver.
Approval remains withheld: current-head Balanced Copilot does not explicitly recommend approval; 1 unresolved review thread(s); applicable CI check contexts are not all successful; actual current-head pull_request workflows are not all successful. The current-head Build workflow failed on four Ubuntu Python variants. Its representative Ubuntu 3.13 log reports the pytest step timed out after 120 minutes; causes for the other jobs were not individually established. A clean source review or passing local selection does not replace successful actual CI. Failed workflows were not rerun.
Posted by thinkall-agent-auto-reviewer
This branch has not been deployed
No deployments
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
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.
Li Jiang (thinkall)
left a comment
Why are these changes needed?
ts_forecastexpects the timestamp to be a column (time_col, which defaults to the first column). When the timestamps are passed as the pandasDataFrameindex instead — the idiomatic shape, and what the reporters of #1229 and of the audit in #1570 (item A1) ran into —TimeSeriesTask.validate_datainferredtime_colfromdataframe.columns[0], i.e. a value column that is frequently the label itself, and the real timestamps were never read.What that looks like on
main, withpandas>=2.0,numpy:time_colis inferred as'y'.pd.to_datetime(df['y'])reads numeric label values as nanoseconds since epoch (1970-01-01), collapsing rows and throwing misleading errors (AssertionError: Duplicate timestamp valuesorValueError: pandas dtypes must be int, float or bool).Solution & Scope of Changes
Consistent DatetimeIndex Promotion:
time_colis inferred anddataframe.indexis aDatetimeIndexwhile the candidate column is not datetime-like, promote the index to a column before timestamp conversion (_promote_datetime_index_if_needed).X_val), and prediction inputs (X).""and integer0) are preserved (df.index.name is Nonefallback to"ds"), ensuring prediction-form equivalence between raw-index frames andreset_index()frames.ds_1,ds_2).Target Alignment & Compatibility:
pd.SeriesandRangeIndextargets.DatetimeIndex, alignment is validated and paired by label (reindex), raisingValueErroron label mismatches to prevent silent data corruption.time_colis never overridden; existing valid date/timestamp columns (including convertible date strings) are preserved.pd.DataFrameso supported NumPy feature arrays reach normalization without premature rejection.DatetimeIndexon prediction outputs across all estimators, including estimators that reset output index (e.g.Orbit,Prophet).Related issue number
Part of #1570 (Track A, item A1).
Tests & Runtime Validation
Comprehensive test suite in
test/automl/test_ts_forecast_datetime_index.pywith 41 dedicated tests covering all input permutations, edge cases, and runtime behaviors:test_unnamed_datetime_index_becomes_the_time_columntest_named_datetime_index_keeps_its_nametest_promoted_index_keeps_every_training_rowtest_index_name_collision_resolves_to_unique_columntest_explicit_index_name_prediction_form_equivalence(parameterized for"",0,"when")test_explicit_time_col_still_wins_over_the_indextest_genuine_missing_timestamp_still_raisestest_timestamp_column_input_is_unchangedtest_date_string_time_col_preserved_over_datetime_indextest_explicit_missing_time_col_rejected_even_with_datetime_indextest_explicit_time_col_missing_from_features_rejected_even_if_present_in_targetstest_xtrain_ytrain_with_datetime_indextest_validation_data_with_datetime_indextest_validation_data_with_dataframe_target_and_datetime_indextest_numpy_array_training_input_with_explicit_time_coltest_numpy_array_validation_input_acceptedtest_training_with_reordered_dataframe_target_aligns_by_labeltest_validation_with_reordered_dataframe_target_aligns_by_labeltest_mismatched_target_index_raises_value_errortest_training_with_range_index_series_targettest_validation_with_range_index_series_targettest_training_with_reordered_series_target_aligns_by_labeltest_training_with_range_index_x_datetime_col_and_datetime_index_series_targettest_validation_with_range_index_x_datetime_col_and_datetime_index_series_targettest_dataframe_target_with_range_index_preserves_ordertest_predict_with_datetime_indextest_predict_restores_datetime_index_preventing_assignment_nanstest_predict_restores_datetime_index_for_estimator_that_resets_output_indexTest Suite Execution
Result:
41 passed in 147.33sCI Build Matrix
All 15 GitHub Actions workflow jobs completed with
SUCCESS:build (ubuntu-latest, 3.10 - 3.13): Passedbuild (windows-latest, 3.10 - 3.13): Passedbuild (windows-11-arm, 3.11 - 3.13): Passedpre-commit-check: Passedchecks: Passedlicense/cla: PassedChecks
time_colremains optional and behavior is backwards-compatible).