-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Modify set_global_logger_level use of contextmanager #418
Conversation
qlib/log.py
Outdated
@@ -165,8 +165,16 @@ def filter(self, record): | |||
return allow | |||
|
|||
|
|||
@contextmanager |
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.
@zhupr Please keep the previous set_global_logger_level
function and create a new contextmanager
.
I think they are both very useful.
Besides, please add detailed docstrings about the feature.
Thanks
qlib/log.py
Outdated
@@ -166,7 +166,79 @@ def filter(self, record): | |||
|
|||
|
|||
def set_global_logger_level(level: int): |
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.
@zhupr You can add a parameter like return_orig_handler_level=False
to control the return value to reuse this function in set_global_logger_level_contextmanager
.
qlib/log.py
Outdated
|
||
|
||
@contextmanager | ||
def set_global_logger_level_contextmanager(level: int): |
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.
The function name is too long...
You can use a shorter name (e.g. use cm
to replace contextmanger)
ffd33c9
to
76c5c5d
Compare
Modify set_global_logger_level use of contextmanager
Description
Motivation and Context
How Has This Been Tested?
pytest qlib/tests/test_all_pipeline.py
under upper directory ofqlib
.Screenshots of Test Results (if appropriate):
Types of changes