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

Remove _study_id parameter from Trial class #4811

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

adjeiv
Copy link
Contributor

@adjeiv adjeiv commented Jul 15, 2023

Motivation

Referencing issue #4678
All uses of trial._study_id were removed in optuna/optuna-dashboard/pull/463, which was merged into the main optuna repository.

Description of the changes

Remove the _study_id parameter from the Trial class, and remove the test that checks the _study_id parameter was always the same as study._study_id.

@github-actions github-actions bot added the optuna.trial Related to the `optuna.trial` submodule. This is automatically labeled by github-actions. label Jul 15, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #4811 (84f848b) into master (effefd7) will increase coverage by 0.01%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #4811      +/-   ##
==========================================
+ Coverage   90.67%   90.69%   +0.01%     
==========================================
  Files         195      195              
  Lines       14602    14601       -1     
==========================================
+ Hits        13241    13242       +1     
+ Misses       1361     1359       -2     
Impacted Files Coverage Δ
optuna/trial/_trial.py 95.65% <ø> (-0.03%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@HideakiImamura
Copy link
Member

Thank you for the PR. I believe the CI error is not caused by the changes in this PR, so let me rerun it.

@HideakiImamura HideakiImamura self-assigned this Jul 18, 2023
@HideakiImamura HideakiImamura added the code-fix Change that does not change the behavior, such as code refactoring. label Jul 18, 2023
@HideakiImamura
Copy link
Member

@Alnusjaponica Could you review this PR?

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.

Copy link
Collaborator

@Alnusjaponica Alnusjaponica 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 Could you merge this PR?

@Alnusjaponica Alnusjaponica removed their assignment Jul 21, 2023
@HideakiImamura HideakiImamura merged commit 222194a into optuna:master Jul 21, 2023
33 checks passed
@HideakiImamura HideakiImamura added this to the v3.3.0 milestone Jul 21, 2023
@adjeiv adjeiv deleted the study_id_param branch October 21, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-fix Change that does not change the behavior, such as code refactoring. optuna.trial Related to the `optuna.trial` submodule. This is automatically labeled by github-actions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants