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

[MNT] update statsforecast version in forecasting extra #6064

Merged
merged 2 commits into from Mar 7, 2024

Conversation

yarnabrina
Copy link
Collaborator

PR #5920 intended to update bounds for statsforecast in all dependency specifications. As noted in #6057, it was done only in all_extras and all_extras_pandas2. This PR updates it for the missed single component optional dependency forecasting.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 5, 2024

why did dependabot not catch this?

@fkiraly fkiraly added the maintenance Continuous integration, unit testing & package distribution label Mar 5, 2024
@yarnabrina
Copy link
Collaborator Author

I don't know why, but I noticed it never does for dependencies that have python version markers.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 5, 2024

strange, but interesting observation

@fkiraly fkiraly added the module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting label Mar 6, 2024
@yarnabrina
Copy link
Collaborator Author

CI took more than 18 hours, but it's finished finally. Ready for review+merge.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 7, 2024

... why did it take 18 hours???

Any relation to the change made by this PR? Or is it more an instance of "new CI is less optimal in the testing everything case"?

@yarnabrina
Copy link
Collaborator Author

yarnabrina commented Mar 7, 2024

All jobs were queued for about 10 hours. I don't know why, but don't think it's caused by new CI or this PR changes.

is it more an instance of "new CI is less optimal in the testing everything case"?

Total runtime wise, sure, but it checks with different (more up to date) versions so that probably should also be kept in mind. As you explained earlier, job creations and setups takes a very significant time.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 7, 2024

As you explained earlier, job creations and setups takes a very significant time.

We should somehow address this, for PRs like this. Clogging the CI pipeline for half a day is not good.

@yarnabrina
Copy link
Collaborator Author

Agreed, triggering all tests even though mostly they do nothing but install is the culprit, and will happen for all PR's that modify pyproject.toml even slightly (I think, though I'm not 100% sure on that). Other than waiting for #5477 or #5719, not sure what to do, and those will need some good focused time which I currently lack.

@fkiraly fkiraly merged commit d945457 into sktime:main Mar 7, 2024
202 checks passed
@yarnabrina yarnabrina deleted the add-missed-dependency-update branch March 7, 2024 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Continuous integration, unit testing & package distribution module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants