Skip to content

ENH: Add min_timeshift support to make_forecasting_frame - #1121

Merged
nils-braun merged 4 commits into
blue-yonder:mainfrom
Lulloooo:fix-min-timeshift-forecasting-frame
May 31, 2026
Merged

nils-braun merged 4 commits into
blue-yonder:mainfrom
Lulloooo:fix-min-timeshift-forecasting-frame

Conversation

@Lulloooo

Copy link
Copy Markdown
Contributor

Summary

This PR introduces a min_timeshift parameter to make_forecasting_frame to allow filtering out early time steps in forecasting frame generation.

Why

As highlighted in issue #992, 'make_forecasting_frame' was missing a 'min_timeshift' parameters. I saw this issue was still open and implemented the suggested enhancement.

Files changed

  • tsfresh/utilities/dataframe_functions.py: added min_timeshift filtering logic
  • tests/units/utilities/test_dataframe_functions.py: added unit test for min_timeshift

Function enhancement

  • Added min_timeshift parameter to make_forecasting_frame.
  • 'min_timeshift' defaults to 0 for backward compatibility
  • Filtered out generated rows where time index < 'min_timeshift'
  • Updated unit test to validate filtering behavior

Testing

  • Verified correct filtering behavior using synthetic time series and boundary checks
  • All unit tests pass locally (pytest)

Notes

  • .gitignore updated to exclude .venv/ during local development

Fixes #992

@nils-braun nils-braun 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.

Well done @Lulloooo and welcome! PR already looks quite good, just a few finishing touches needed

# Check fewer rows (core behavior)
assert len(df_filtered) < len(df_default)
# Check target is aligned
assert len(y_filtered) == len(df_filtered.groupby("id").size())

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.

isn't this the same as:

Suggested change
assert len(y_filtered) == len(df_filtered.groupby("id").size())
assert len(y_filtered) == df_filtered.nunique("id")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implemented your suggestion in the last commit.

min_timeshift=2,
)
# Check fewer rows (core behavior)
assert len(df_filtered) < len(df_default)

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.

As you know the data, it might be better to check the actual len

But using the default df & y, you can check whether the two equal on the intersection

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a more robust test in the latest commit, comparing the intersection between default and filtered outputs as suggested. Thanks for the review!

@Lulloooo

Copy link
Copy Markdown
Contributor Author

CI is failing during installation (pkg_resources missing in build step) across all Python versions. It seems unrelated to the changes in this PR. Happy to help if needed. Thank you!

@nils-braun

Copy link
Copy Markdown
Collaborator

Yes, correct! There is a PR to fix this problem (#1120). Once it is in, your PR will also succeed to build again.

@nils-braun
nils-braun enabled auto-merge (squash) May 31, 2026 18:13
@nils-braun
nils-braun merged commit db3fb56 into blue-yonder:main May 31, 2026
5 checks passed
@nils-braun

Copy link
Copy Markdown
Collaborator

Thanks again! I have just released v0.21.2 with your feature.

@Lulloooo

Copy link
Copy Markdown
Contributor Author

Glad to help! Looking forward to contributing more if something that fits my skills comes up.😁

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.

Why make_forecasting_frame does not have min_timeshift argument?

2 participants