Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: append_values and prepend_values correctly extends RangeIndex-typed TimeSeries #1435

Merged
merged 2 commits into from
Dec 19, 2022

Conversation

mabilton
Copy link
Contributor

Fixes #1434.

Summary

Previously, append_values and prepend_values did not correctly handle extending RangeIndex-typed TimeSeries. For example, the following code snippet:

from darts.utils.timeseries_generation import linear_timeseries
import numpy as np

series = linear_timeseries(start=1, length=5, freq=2)
print('Before `append_values`:')
print(list(series.time_index))
new_values = np.ones((1,))
print('After `append_values`:')
print(list(series.append_values(new_values).time_index))

would print:

Before `append_values`:
[1, 3, 5, 7, 9]
After `append_values`:
[0, 1, 2, 3, 4, 5]

This has been fixed in this PR so that this code now prints:

Before `append_values`:
[1, 3, 5, 7, 9]
After `append_values`:
[1, 3, 5, 7, 9, 11]

Similarly, the code snippet:

from darts.utils.timeseries_generation import linear_timeseries
import numpy as np

series = linear_timeseries(start=1, length=5, freq=2)
print('Before `prepend_values`:')
print(list(series.time_index))
new_values = np.ones((1,))
print('After `prepend_values`:')
print(list(series.prepend_values(new_values).time_index))

would previously print:

Before `append_values`:
[1, 3, 5, 7, 9]
After `append_values`:
[0, 1, 2, 3, 4, 5]

Now, this same code snippet prints:

Before `prepend_values`:
[1, 3, 5, 7, 9]
After `prepend_values`:
[-1, 1, 3, 5, 7, 9]

Other Information

Tests have also been updated to explicitly check for this behaviour.

@codecov-commenter
Copy link

codecov-commenter commented Dec 18, 2022

Codecov Report

Base: 93.80% // Head: 93.80% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (1e17993) compared to base (9a40ca6).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1435   +/-   ##
=======================================
  Coverage   93.80%   93.80%           
=======================================
  Files          94       94           
  Lines        9438     9423   -15     
=======================================
- Hits         8853     8839   -14     
+ Misses        585      584    -1     
Impacted Files Coverage Δ
darts/timeseries.py 91.92% <ø> (+0.01%) ⬆️
...arts/models/forecasting/torch_forecasting_model.py 89.50% <0.00%> (-0.05%) ⬇️
darts/models/forecasting/block_rnn_model.py 98.24% <0.00%> (-0.04%) ⬇️
darts/models/forecasting/nhits.py 99.27% <0.00%> (-0.01%) ⬇️
darts/datasets/__init__.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@hrzn hrzn left a comment

Choose a reason for hiding this comment

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

Great, thanks a lot 👍

@hrzn hrzn merged commit 705ec0b into unit8co:master Dec 19, 2022
@mabilton mabilton deleted the fix/append_values branch December 27, 2022 06:42
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.

[BUG] append_values incorrectly extends time_index of pd.RangeIndex-typed TimeSeries
3 participants