fix colorlog compatibility problem#4406
Conversation
1d6c760 to
4ff5fd4
Compare
toshihikoyanase
left a comment
There was a problem hiding this comment.
Thank you for your PR. I have two comments.
optuna/logging.py
Outdated
|
|
||
|
|
||
| def create_default_formatter() -> colorlog.ColoredFormatter: | ||
| def create_default_formatter() -> Union[Formatter, colorlog.ColoredFormatter]: |
There was a problem hiding this comment.
colorlog.ColoredFormatter extends logging.Formatter, and I guess the return type can be Formatter.
| def create_default_formatter() -> Union[Formatter, colorlog.ColoredFormatter]: | |
| def create_default_formatter() -> Formatter: |
And, the returned value is pass to logging.Handler.setFormatter, and it expects logging.Formatter. So, I don't think it does not depend on the colorlog version.
Line 70 in ea55b34
|
@contramundum53 Could you review this PR as an author of #4403? |
|
I confirmed that it works with colorlog 4.8. Thanks for the PR! |
Co-authored-by: Toshihiko Yanase <toshihiko.yanase@gmail.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #4406 +/- ##
==========================================
- Coverage 90.41% 90.40% -0.02%
==========================================
Files 172 172
Lines 13675 13679 +4
==========================================
+ Hits 12364 12366 +2
- Misses 1311 1313 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@keisuke-umezawa Could you review this PR? |
Co-authored-by: Toshihiko Yanase <toshihiko.yanase@gmail.com>
keisuke-umezawa
left a comment
There was a problem hiding this comment.
LGTM! I confirmed it works on my local env for both situations. Thank you for your quick fix.
Motivation
Fix #4403.
Description of the changes
Use logging.Formatter if
_color_supportedreturns True.