Skip to content

Fix/warn when extracting stochastic TimeSeries values - #3307

Open
Pallav-Biyala wants to merge 4 commits into
unit8co:masterfrom
Pallav-Biyala:fix/warn-stochastic-values
Open

Pallav-Biyala wants to merge 4 commits into
unit8co:masterfrom
Pallav-Biyala:fix/warn-stochastic-values

Conversation

@Pallav-Biyala

Copy link
Copy Markdown

Fixes #3296

Summary

  • Changed the default value of the sample parameter in TimeSeries.values() from 0 to None to distinguish between callers who omit the argument and those who explicitly select a sample.
  • When sample is omitted for a stochastic time series, emit a warning explaining that only sample 0 will be returned and suggesting TimeSeries.mean() or TimeSeries.quantile(0.5) when a point forecast is desired.
  • Preserve backward-compatible behavior: calling values() still returns sample 0 by default. Explicitly specifying sample=0 or another sample does not trigger the warning.
  • Updated the method's docstring to document the new default and warning behavior.
  • Added four tests covering the warning and returned values for stochastic time series, explicitly specified samples, and deterministic time series.
  • No changes were needed to TimeSeries.to_dataframe(), which already represents samples of a stochastic time series in separate columns.

Testing

  • darts/tests/test_timeseries.py: 329 passed, 1 skipped.
  • git diff --check: passed.

Signed-off-by: Pallav-Biyala <pallav.biyala@iitgn.ac.in>
Signed-off-by: Pallav-Biyala <pallav.biyala@iitgn.ac.in>

@dennisbader dennisbader left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix and your first contribution @Pallav-Biyala 🚀

I slightly updated the warning message and tests. Apart from that everything looks great 💯

I'll merge once all tests have passed

This branch has not been deployed

No deployments
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.

Warn or provide aggregation parameter in TimeSeries.values() / to_dataframe() when called on stochastic TimeSeries (n_samples > 1)

2 participants