Skip to content
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

refactor: set log level to separate out train args #314

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anhuong
Copy link
Collaborator

@anhuong anhuong commented Aug 23, 2024

Description of the change

Separate out python logging and setting log level with setting log level using train_args and altering train_args.log_level to match python logging level.

Related issue number

How to verify the PR

Was the PR tested

  • I have added >=1 unit test(s) for every new method I have added.
  • I have ensured all unit tests pass

- separate out the train args from python logger

Signed-off-by: Anh Uong <[email protected]>
Comment on lines +56 to +61
train_logger = logging.getLogger()
if logger_name:
train_logger = logging.getLogger(logger_name)
else:
train_logger = logging.getLogger()

set_python_log_level(log_level, train_logger)
set_python_log_level(log_level)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have a generic function set_python_log_level to set log level of any logger passed to it from any module of the code. Thank you!

Regarding function set_log_level, according to me, it is called to set root log level in the code based on either CLI using train_argsor ENV variable. Hence, I believe, if we don't want to call set_python_log_level twice in the function set_log_level, we can modify the code in function set_log_level as below:

set_python_log_level(log_level)
train_logger = logging.getLogger()
if logger_name:
    train_logger = logging.getLogger(logger_name)

return train_args, train_logger

Call set_python_log_level(log_level), will set the root log level and train_logger with or without logger_name, will inherit the root log level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants