replace print with logger #1104
Conversation
* Add get_my_logger() * Use logger instead of print * Fix log level * Removed line-breaks for readability * Use setup_logging() * Add rich to requirements.txt * Make simple * Use logger instead of print --------- Co-authored-by: Kohya S <52813779+kohya-ss@users.noreply.github.com>
|
@shirayu Thank you again for your great contribution. I have made the following modifications:
Please take a look at |
kohya-ss
changed the title
library/utils.py
Outdated
| handler = RichHandler() | ||
| except ImportError: | ||
| print("rich is not installed, using basic logging") |
There was a problem hiding this comment.
I think that it is better to use logger.info(....) after logging.root.addHandler(handler) like this.
msg_init = None
# set message on ImportError
msg_init = "rich is not installed, using basic logging"
....
logging.root.addHandler(handler)
if msg_init is not None:
logger = logging.getLogger(__name__)
logger.info(msg_init)There was a problem hiding this comment.
Thanks for the great suggestion! I will adopt this one.
fine_tune.py
Outdated
|
|
||
|
|
||
| def setup_parser() -> argparse.ArgumentParser: | ||
| parser = argparse.ArgumentParser() | ||
|
|
||
| add_logging_arguments(parser) # モジュール指定がないのがちょっと気持ち悪い / bit weird that this does not have module prefix |
There was a problem hiding this comment.
Moving add_logging_arguments() and setup_logging() from library/utils.py to library/logger_utils/__init__.py is an option.
The call will be like this.
logger_utils.add_logging_arguments(parser)There was a problem hiding this comment.
Thank you! I think this is certainly one way to do it. I just don't want to add too many files, so I guess I'll just leave it as it was.
library/utils.py
Outdated
| from rich.logging import RichHandler | ||
|
|
||
| handler = RichHandler() |
There was a problem hiding this comment.
I would like to add one more thing.
By default, rich logs to standard output (stdout), but I think it is better to change it as follows.
Proposal
from rich.console import Console
from rich.logging import RichHandler
handler = RichHandler(console=Console(stderr=True))There was a problem hiding this comment.
Thank you! That certainly seems to be consistent.
If someone wants the output to be on stdout as before, there is an option for it.
No description provided.