Conversation
…gration to optuna_integration
|
@y0z Could you review this PR? |
y0z
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
I leave a review comment.
nabenabe0928
left a comment
There was a problem hiding this comment.
Thank you for your PR, please address my comments:)
|
@buruzaemon Basically, the conflict is happening in pyproject.toml, but you can keep both torch and xgboost in pyproject.toml. After addressing my comments and the conflict resolve, we can merge this PR! |
|
... and finally, resolved that conflict in pyproject.toml. ptal |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
==========================================
- Coverage 65.06% 64.92% -0.14%
==========================================
Files 25 26 +1
Lines 1869 1933 +64
==========================================
+ Hits 1216 1255 +39
- Misses 653 678 +25 ☔ View full report in Codecov by Sentry. |
|
Could you please apply isort? |
sure, i am on that now... |
nabenabe0928
left a comment
There was a problem hiding this comment.
@buruzaemon
Here are the changes you should make by isort.
tests/test_xgboost.py
Outdated
| import numpy as np | ||
| import pytest | ||
|
|
||
| import optuna | ||
| from optuna_integration._imports import try_import | ||
| from optuna_integration.xgboost import XGBoostPruningCallback | ||
| from optuna.testing.pruners import DeterministicPruner |
There was a problem hiding this comment.
| import numpy as np | |
| import pytest | |
| import optuna | |
| from optuna_integration._imports import try_import | |
| from optuna_integration.xgboost import XGBoostPruningCallback | |
| from optuna.testing.pruners import DeterministicPruner | |
| import numpy as np | |
| import optuna | |
| from optuna.testing.pruners import DeterministicPruner | |
| import pytest | |
| from optuna_integration._imports import try_import | |
| from optuna_integration.xgboost import XGBoostPruningCallback |
There was a problem hiding this comment.
... and this is done!
| from typing import Any | ||
|
|
||
| import optuna | ||
| import optuna_integration |
There was a problem hiding this comment.
| from typing import Any | |
| import optuna | |
| import optuna_integration | |
| from typing import Any | |
| import optuna | |
| import optuna_integration |
There was a problem hiding this comment.
... and this change to the imports is done as well!
nabenabe0928
left a comment
There was a problem hiding this comment.
Thank you for the changes, LGTM!
Motivation
Description of the changes