-
-
Notifications
You must be signed in to change notification settings - Fork 88
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
Code fix/download csv #723
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #723 +/- ##
==========================================
+ Coverage 67.20% 67.66% +0.46%
==========================================
Files 35 35
Lines 2293 2329 +36
==========================================
+ Hits 1541 1576 +35
- Misses 752 753 +1 ☔ View full report in Codecov by Sentry. |
@nabenabe0928 @HideakiImamura Could you review this PR? |
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 for the PR, the new feature looks very convenient!
I checked the Python file for now and refactored a bit.
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
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.
I think you should check the all of studies in https://gist.github.com/HideakiImamura/668d6f5703c58255e54ece03cd270f2f.
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
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.
LGTM.
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 for the changes, LGTM!!
I added simple tests, please check below.
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, I added simple tests.
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Co-authored-by: Shuhei Watanabe <[email protected]>
Thank you for adding the tests! I suppose there was another option to separate such expansions into a different PR, but for this instance, I think it's fine to include them in this PR. |
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 for adding the tests, LGTM!
Contributor License Agreement
This repository (
optuna-dashboard
) and Goptuna share common code.This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.
Reference Issues/PRs
I will take over this PR:
#648
What does this implement/fix? Explain your changes.