Closed phi-friday closed 1 month ago
Attention: Patch coverage is 88.88889%
with 2 lines
in your changes missing coverage. Please review.
Project coverage is 96.01%. Comparing base (
aed6a25
) to head (89f65dc
). Report is 3 commits behind head on master.
Files with missing lines | Patch % | Lines |
---|---|---|
bayes_opt/event.py | 90.90% | 1 Missing :warning: |
bayes_opt/logger.py | 80.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hey @phi-friday,
You're right that enums should be used, but the current Logger setup has deeper issues beyond that. The ScreenLogger
, JSONLogger
, etc., aren't working well for several reasons. I’d prefer a full overhaul, moving away from the observer pattern entirely, so from my perspective this PR isn’t worth merging as a quick fix.
i agree.
When I looked at where
Events
is used, I realized that it would be better to inherit fromEnum
.