-
-
Notifications
You must be signed in to change notification settings - Fork 628
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
Add version constraint for Optuna #2163
Add version constraint for Optuna #2163
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for letting us know about the upcoming Optuna API changes!
Once Optuna 3.0 is released, we can upgrade this sweeper to use the new API.
@@ -27,7 +27,7 @@ | |||
], | |||
install_requires=[ | |||
"hydra-core>=1.1.0.dev7", | |||
"optuna>=2.10.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you keep the "optuna>=2.10.0" in the meanwhile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you mean it's enough to track the issue #2162 and not necessary to constrain the version, right? It will show warning messages for users when Optuna v3 is available. Is it OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just throwing this out there: using multiple constraints on the version should be legal:
install_requires=[
...
"optuna >= 2.10.0, < 3.0.0",
...
The downside of pinning Optuna < 3.0 is that users would not be able to access new Optuna features. Jieru, is that why you were concerned about the pin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for the confusion, i should have provided more context. I meant "optuna >= 2.10.0, < 3.0.0"
the current impl assumes the version is >=2.10.0 so it'd be great to keep that still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you @himkt!
See #2162 for more information.
Motivation
This PR updates the version constraint of Optuna to avoid installing the next major version of Optuna.
See also 🔗 #2162
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
(How should this PR be tested? Do you require special setup to run the test or repro the fixed bug?)
Related Issues and PRs
#2162