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.

Sign up for GitHub

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

Jump to bottom

Drop implicit create-study in ask command #5415

Merged
merged 7 commits into from May 21, 2024
Merged

Drop implicit create-study in ask command #5415

merged 7 commits into from May 21, 2024

Conversation

eukaryo
Copy link
Collaborator

@eukaryo eukaryo commented Apr 23, 2024 •
edited

Motivation

I want ask command to stop implicit create-study.

Description of the changes

  • I replaced code of create-study in ask command with a custom error message.
  • I modified related tests.
  • I added a test for this change.

Note

I believe that the usage of the CLI should be similar to the usage of the python library. When using optuna in python, user must call optuna.create_study method explicitly. Therefore, I want to introduce this change.

@eukaryo eukaryo added the compatibility Change that breaks compatibility. label Apr 23, 2024
@eukaryo eukaryo added this to the v4.0.0 milestone Apr 23, 2024
not522
not522 reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

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

Could you add tests to confirm that that ask command fails on non-exist studies?

optuna/cli.py Outdated Show resolved Hide resolved
not522
not522 approved these changes Apr 26, 2024
View reviewed changes
Copy link
Member

@not522 not522 left a comment •
edited

Choose a reason for hiding this comment

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

Thank you for your update. LGTM.

@not522 not522 removed their assignment Apr 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2024

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label May 8, 2024
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label May 8, 2024
HideakiImamura
HideakiImamura reviewed May 9, 2024
View reviewed changes
Copy link
Member

@HideakiImamura HideakiImamura left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I have a comment. PTAL.

tests/test_cli.py Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

This pull request has not seen any recent activity.

@github-actions github-actions bot added the stale Exempt from stale bot labeling. label May 16, 2024
@nabenabe0928 nabenabe0928 removed the stale Exempt from stale bot labeling. label May 17, 2024
HideakiImamura
HideakiImamura approved these changes May 21, 2024
View reviewed changes
Copy link
Member

@HideakiImamura HideakiImamura left a comment

Choose a reason for hiding this comment

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

LGTM.

@HideakiImamura HideakiImamura merged commit fe400d1 into optuna:master May 21, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@not522 not522 not522 approved these changes

@HideakiImamura HideakiImamura HideakiImamura approved these changes

Assignees

@HideakiImamura HideakiImamura

Labels
compatibility Change that breaks compatibility.
Projects
None yet
Milestone
v4.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@eukaryo @not522 @HideakiImamura @nabenabe0928