Skip to content

Fix type of n_windows in cross_validation#1378

Merged
marcopeix merged 2 commits intoNixtla:mainfrom
Riklia:fix/n_windows_type
Jan 12, 2026
Merged

Fix type of n_windows in cross_validation#1378
marcopeix merged 2 commits intoNixtla:mainfrom
Riklia:fix/n_windows_type

Conversation

@Riklia
Copy link
Contributor

@Riklia Riklia commented Aug 31, 2025

As described in the documentation and in the implementation, when test_size is defined, we set n_windows to None. However, the type of n_windows is specified as int. This causes a warning on the caller side.

@CLAassistant
Copy link

CLAassistant commented Aug 31, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@marcopeix
Copy link
Contributor

Hi @Riklia, I want to make a release of neuralforecast soon and would like to include your PR. Can you solve the conflict please? Thanks!

@Riklia Riklia force-pushed the fix/n_windows_type branch 2 times, most recently from 43688bc to b82c40e Compare January 8, 2026 21:15
@Riklia
Copy link
Contributor Author

Riklia commented Jan 8, 2026

Hi @marcopeix! I rebased, and it should be fine now.

Copy link
Contributor

@marcopeix marcopeix left a comment

Choose a reason for hiding this comment

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

Hello @Riklia , can you address these comments please? Also, make sure to install the pre-commit hooks with:

pre-commit install
pre-commit run --files neuralforecast/*

Linting errors should be picked up before you make a PR.

Thanks!

@Riklia Riklia force-pushed the fix/n_windows_type branch from b82c40e to a2ddb14 Compare January 9, 2026 19:45
Add assertion checks to satisfy mypy and prevent invalid values from propagating into the validation logic.
@Riklia Riklia force-pushed the fix/n_windows_type branch from a2ddb14 to 49b104b Compare January 9, 2026 19:54
@Riklia
Copy link
Contributor Author

Riklia commented Jan 9, 2026

Thanks for the detailed instructions! Addressed all the comments.

@marcopeix marcopeix self-requested a review January 12, 2026 17:52
@marcopeix marcopeix merged commit 10412f5 into Nixtla:main Jan 12, 2026
15 checks passed
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.

3 participants