-
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
Update qlib logger #393
Update qlib logger #393
Conversation
qlib/log.py
Outdated
super().__init__(name, bases, dic) | ||
|
||
def __new__(cls, name, bases, dict): | ||
wrapper_dict = type(logging.getLogger("MetaLogger")).__dict__.copy() |
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.
Why not use Logger directly?
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.
type(logging.Logger)
will give the result of <class 'type'>
, while calling type(logger object)
will give the result <class 'logging.Logger'>
. The aim of wrapper dict here aimed to get the type dictionary of logger object which needs a logger instance here (since we need the information and messages in the dict for making help
work).
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.
Will it be different by replacing type(logging.getLogger("MetaLogger"))
with logging.Logger
?
Why should we create a Logger instance here?
qlib/log.py
Outdated
def __new__(cls, name, bases, dict): | ||
wrapper_dict = type(logging.getLogger("MetaLogger")).__dict__.copy() | ||
wrapper_dict.update(dict) | ||
wrapper_dict["__doc__"] = logging.getLogger("MetaLogger").__doc__ |
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.
Why not use logging.Logger.__doc__
?
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.
Yes, we can use logging.Logger.__doc__
here, which will have the same results as doing so with an instance.
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.
Why should we create a Logger instance here instead of using the class?
Using the class directly takes fewer steps and the code looks simpler
Update qlib logger
Description
The first commit solves the problem of pickling logger object under certain python version. But, I'm still figuring out how to maintain the original docstring of the logger.
First trial: now use
help
function will print all the docstrings ofQlibLogger
, but the docstring hints won't show up while using IDEs such as VSCode.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