feat(automl): add backtesting charts to AutoML time series inference notebook#132
Merged
openshift-merge-bot[bot] merged 7 commits intoJun 17, 2026
Conversation
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthrough
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
LukaszCmielowski
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/training/automl/shared/back_testing_charts.py`:
- Around line 16-21: Add error handling to the `_matplotlib()` function to
gracefully handle missing matplotlib dependencies. Wrap the import statements
for matplotlib.dates and matplotlib.pyplot in a try/except block that catches
ImportError, and when caught, raise a new ImportError with a clear message
indicating that matplotlib is required for chart rendering and should be
installed via the requirements.txt file. This aligns with the existing error
handling pattern used in other functions like `_present_frame()` for optional
imports.
In `@components/training/automl/shared/back_testing.py`:
- Around line 156-159: The _closest() function in the code independently selects
the closest quantile level to both 0.1 and 0.9 targets. When only one quantile
column exists, this results in both the lower and upper bounds being selected as
the same value, causing inconsistency. Modify the logic to select the lower
quantile first by calling _closest(0.1), then for the upper quantile, select
from the remaining levels (excluding the already-selected lower quantile) before
calling _closest(0.9). This ensures that when multiple quantiles are available,
lower and upper bounds are always distinct, and when only one exists, you can
handle it as a special case or raise an appropriate error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ac28aaa1-bcea-4472-b67a-0e67d96f6a95
📒 Files selected for processing (8)
components/training/automl/autogluon_timeseries_models_training/README.mdcomponents/training/automl/autogluon_timeseries_models_training/component.pycomponents/training/automl/autogluon_timeseries_models_training/metadata.yamlcomponents/training/automl/shared/back_testing.pycomponents/training/automl/shared/back_testing_charts.pycomponents/training/automl/shared/notebook_templates/timeseries_notebook.ipynbcomponents/training/automl/shared/tests/test_back_testing.pycomponents/training/automl/shared/tests/test_back_testing_charts.py
DorotaDR
suggested changes
Jun 16, 2026
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
components/training/automl/shared/back_testing_charts.py (3)
287-290:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing error handling for malformed
cutoff_starttimestamp.
pd.to_datetime(cutoff_start)will raise ValueError on invalid timestamps. Ifcutoff_startcomes from untrustedback_testing.json, wrap in try/except or validate input.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/training/automl/shared/back_testing_charts.py` around lines 287 - 290, The _draw_cutoff function does not handle invalid timestamp formats in the cutoff_start parameter, which will cause pd.to_datetime(cutoff_start) to raise a ValueError if the input is malformed. Wrap the pd.to_datetime(cutoff_start) call in a try/except block to gracefully handle ValueError exceptions, and either log a warning and return early, or skip drawing the cutoff line if the timestamp cannot be parsed. This ensures the function is resilient to invalid timestamps from untrusted sources like back_testing.json.
81-87:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing validation for untrusted
forecast_datastructure.
pd.DataFrame(forecast_data)andpd.to_datetime(frame["timestamp"])will raise exceptions on malformed input. Ifforecast_datacomes from untrusted sources (e.g., user-uploadedback_testing.json), add validation or wrap in try/except to prevent crashes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/training/automl/shared/back_testing_charts.py` around lines 81 - 87, The forecast_data_to_frame function lacks error handling for malformed input from potentially untrusted sources like user-uploaded back_testing.json files. Add validation or wrap the pd.DataFrame(forecast_data) call and the pd.to_datetime(frame["timestamp"]) call in try/except blocks to gracefully handle cases where the input structure is invalid, the required columns are missing, or the timestamp cannot be parsed. Provide meaningful error handling that either returns a valid empty DataFrame, logs the error details, or raises a more informative exception rather than allowing the raw pandas exceptions to propagate.
371-379:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow exception handling may miss timestamp and structure errors.
The try/except only catches
ValueErrorfrom_draw_forecast(). Malformed timestamps or invalid data structures will raiseTypeError,KeyError, or pandas exceptions. Either broaden the exception type or add upstream validation inforecast_data_to_frame().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/training/automl/shared/back_testing_charts.py` around lines 371 - 379, The exception handling around the _draw_forecast() call is too narrow, catching only ValueError while malformed timestamps or invalid data structures will raise TypeError, KeyError, or pandas exceptions. Broaden the except clause to catch multiple exception types (ValueError, TypeError, KeyError, and pandas exceptions like pandas.errors.ParserError) or add upstream validation in forecast_data_to_frame() to ensure the data is properly validated before being passed to _draw_forecast(), preventing these errors from occurring in the first place. Choose whichever approach aligns with your error handling strategy.
♻️ Duplicate comments (1)
components/training/automl/shared/back_testing_charts.py (1)
16-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing ImportError handling remains unaddressed.
Past review flagged the lack of try/except coverage in
_matplotlib(). If matplotlib is missing,render_back_testing_charts()will crash at line 344. Add graceful fallback or wrap the call site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/training/automl/shared/back_testing_charts.py` around lines 16 - 21, The `_matplotlib()` function lacks error handling for missing matplotlib dependencies, causing `render_back_testing_charts()` to crash if matplotlib is not installed. Add a try/except block in the `_matplotlib()` function to catch ImportError when importing matplotlib.dates and matplotlib.pyplot, and either raise a more informative error or return None to indicate failure. Alternatively, wrap the call to `_matplotlib()` within `render_back_testing_charts()` with try/except handling to gracefully handle the missing dependency and skip chart rendering or provide a user-friendly error message.
🧹 Nitpick comments (1)
components/training/automl/shared/tests/test_back_testing_charts.py (1)
268-315: 💤 Low valueTest validates fixed date format that strips hour information.
Line 309 asserts
DateFormatteris used, which validates the"%m-%d"format on line 177 ofback_testing_charts.py. Past review flagged this format as problematic for hourly datasets. The test correctly verifies the implementation but locks in potentially incorrect behavior. Consider updating the test to verify default date formatting instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/training/automl/shared/tests/test_back_testing_charts.py` around lines 268 - 315, The test_plot_timeseries_forecasts_styles_date_axis test is validating a fixed date format ("%m-%d") that strips hour information, which is problematic for hourly datasets. Update the assertion on line 309 that checks for DateFormatter (the isinstance check for axis.xaxis.get_major_formatter()) to instead verify that default date formatting is used, which would appropriately preserve time information for hourly datasets rather than locking in the potentially incorrect fixed format behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/training/automl/shared/back_testing_charts.py`:
- Around line 183-184: The _target_column_name function attempts to access the
first column of a DataFrame without validating that columns exist, which will
raise an IndexError if the DataFrame is empty. Add a validation check at the
beginning of the _target_column_name function to ensure data.columns is not
empty, and either raise a descriptive ValueError or return a default value if no
columns are present, before attempting to access data.columns[0].
In
`@components/training/automl/shared/notebook_templates/timeseries_notebook.ipynb`:
- Around line 283-289: The code on line 289 calls
render_back_testing_forecast_charts(back_testing) without verifying that the
back_testing variable exists. Add a check for "back_testing" in globals() to the
conditional logic. If back_testing does not exist (i.e., the Back-testing
metrics cell was skipped or failed), print a message indicating the variable is
not available instead of attempting to use it and raising a NameError.
- Around line 257-262: Add JSON structure validation at both locations where
back_testing.json is loaded. At
components/training/automl/shared/notebook_templates/timeseries_notebook.ipynb
lines 257-262 (anchor site with render_back_testing_metrics call) and lines
433-438 (sibling site in plot cell), validate the loaded JSON dictionary against
the expected schema before passing it to rendering functions. You can either add
a validation function that checks for required keys and types once before each
render call, or wrap each json.load and subsequent render call in a try/except
block to catch and gracefully handle malformed data.
---
Outside diff comments:
In `@components/training/automl/shared/back_testing_charts.py`:
- Around line 287-290: The _draw_cutoff function does not handle invalid
timestamp formats in the cutoff_start parameter, which will cause
pd.to_datetime(cutoff_start) to raise a ValueError if the input is malformed.
Wrap the pd.to_datetime(cutoff_start) call in a try/except block to gracefully
handle ValueError exceptions, and either log a warning and return early, or skip
drawing the cutoff line if the timestamp cannot be parsed. This ensures the
function is resilient to invalid timestamps from untrusted sources like
back_testing.json.
- Around line 81-87: The forecast_data_to_frame function lacks error handling
for malformed input from potentially untrusted sources like user-uploaded
back_testing.json files. Add validation or wrap the pd.DataFrame(forecast_data)
call and the pd.to_datetime(frame["timestamp"]) call in try/except blocks to
gracefully handle cases where the input structure is invalid, the required
columns are missing, or the timestamp cannot be parsed. Provide meaningful error
handling that either returns a valid empty DataFrame, logs the error details, or
raises a more informative exception rather than allowing the raw pandas
exceptions to propagate.
- Around line 371-379: The exception handling around the _draw_forecast() call
is too narrow, catching only ValueError while malformed timestamps or invalid
data structures will raise TypeError, KeyError, or pandas exceptions. Broaden
the except clause to catch multiple exception types (ValueError, TypeError,
KeyError, and pandas exceptions like pandas.errors.ParserError) or add upstream
validation in forecast_data_to_frame() to ensure the data is properly validated
before being passed to _draw_forecast(), preventing these errors from occurring
in the first place. Choose whichever approach aligns with your error handling
strategy.
---
Duplicate comments:
In `@components/training/automl/shared/back_testing_charts.py`:
- Around line 16-21: The `_matplotlib()` function lacks error handling for
missing matplotlib dependencies, causing `render_back_testing_charts()` to crash
if matplotlib is not installed. Add a try/except block in the `_matplotlib()`
function to catch ImportError when importing matplotlib.dates and
matplotlib.pyplot, and either raise a more informative error or return None to
indicate failure. Alternatively, wrap the call to `_matplotlib()` within
`render_back_testing_charts()` with try/except handling to gracefully handle the
missing dependency and skip chart rendering or provide a user-friendly error
message.
---
Nitpick comments:
In `@components/training/automl/shared/tests/test_back_testing_charts.py`:
- Around line 268-315: The test_plot_timeseries_forecasts_styles_date_axis test
is validating a fixed date format ("%m-%d") that strips hour information, which
is problematic for hourly datasets. Update the assertion on line 309 that checks
for DateFormatter (the isinstance check for axis.xaxis.get_major_formatter()) to
instead verify that default date formatting is used, which would appropriately
preserve time information for hourly datasets rather than locking in the
potentially incorrect fixed format behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 36c82ebf-04d1-4929-8830-435b328031f3
📒 Files selected for processing (3)
components/training/automl/shared/back_testing_charts.pycomponents/training/automl/shared/notebook_templates/timeseries_notebook.ipynbcomponents/training/automl/shared/tests/test_back_testing_charts.py
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
Author
|
/ok-to-test |
Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
Author
|
/ok-to-test |
DorotaDR
approved these changes
Jun 17, 2026
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DorotaDR The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 84febff
into
opendatahub-io:main
21 of 23 checks passed
LukaszCmielowski
added a commit
to LukaszCmielowski/pipelines-components
that referenced
this pull request
Jun 17, 2026
Resolved conflict in timeseries_data_loader/component.py: - Kept enhanced sample_rows logic with ISO timestamp conversion (from PR opendatahub-io#132) - Kept display_name metadata at start of context (from this PR) - Kept write_outputs status tracking (from PR opendatahub-io#132) Merged changes: - PR opendatahub-io#132: AutoML timeseries notebook backtesting charts - PR opendatahub-io#138: ai4rag 0.6.4 and ogx-client 1.1.0 updates Signed-off-by: Lukasz Cmielowski <lcmielow@redhat.com> Assisted-by: Cursor
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
2 participants
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.
Description of your changes:
Description of your changes:
The time series training pipeline already writes
metrics/back_testing.json, but the generated inference notebook only printed raw JSON — no charts.This PR adds offline matplotlib visualization aligned with the AutoGluon backtesting tutorial:
back_testing_charts.py— shared plotting module (render_back_testing_charts())Cutoff …: MASE = …print lines + table (from existingper_window_metrics)eval_metricacross validation windows (cross-series aggregate, no JSON schema change)matplotlibimports (same pattern as ROC/PR cells in tabular notebooks)timeseries_notebook.ipynb— new “Back-testing charts” section; helpers injected at notebook generation vianotebook_backtest_charts_source()autogluon_timeseries_models_training— wires chart source into notebook placeholder substitutionback_testing.py— cutoff in per-window metrics; P10/P90 quantile bounds in forecast dataNo new runtime dependencies. Works from precomputed artifacts only (no
TimeSeriesPredictorrequired in the notebook).Checklist:
Pre-Submission Checklist
Learn more about the pull request title convention used in this repository.
Additional Checklist Items for New or Updated Components/Pipelines
metadata.yamlincludes freshlastVerifiedtimestampare present and complete
snake_casenaming conventionSummary by CodeRabbit
Summary
New Features
Bug Fixes
Documentation
Tests