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

Force logging setup on import #34

Closed
MaxPensel opened this issue Jan 29, 2021 · 2 comments
Closed

Force logging setup on import #34

MaxPensel opened this issue Jan 29, 2021 · 2 comments

Comments

@MaxPensel
Copy link

Currently, when create_cli is answered with "yes", then the app() function will contain the logging setup. However, when the entry point is not the command line but a notebook, then using something like

from <project>.util import logger
logger.info("Test")

in a notebook cell uses the logger in its default configuration.

Likewise, when you have a function in the project library such as

from <project>.util import logger

def some_func():
    logger.info("Test")

and you import and call this function from a notebook, the logger will be in its default configuration, instead of the config in config/config.yml.

One fix that I have found is adding these two lines to the __init__.py:

from .util import logging_setup, load_config
logging_setup(load_config("../config/config.yml"))

This configures the logger on every import of the project library.

Perhaps this is worth including in the template unless there are some reasons for not putting code in the __init__.py.

@klamann
Copy link
Contributor

klamann commented Jan 29, 2021

I see your point, but this is not how the logger is intended to be used. You should not import the logger instance with from <project>.util import logger but instead use logging.getLogger() to get the static instance for each file individually, like so

logger = logging.getLogger('my_project')

This avoids unnecessary references and circular imports.

About the logging setup: I think it's important to explicitly call the logging setup as part of your application (if you want to use it) because there's a lot of stuff you might want to do before you call the logging setup: evaluating your command line parameters, loading your configuration, handling missing configuration, handling errors in the config, etc.

If we add load_config and logging_setup to __init__.py, we lose control over all these situations. Also, adding complexity to __init__.py is generally a bad idea, because imports are messy at this stage and it is easy to get import errors. And I think it's not bad practice to load the config and call the logging setup at the top of your notebook if you want to use the logging module.

I am in favor of showing how to use the logging module as you suggested in #33 though.

@MaxPensel
Copy link
Author

Makes perfect sense! Thank's for the extensive explanation, very educational! :)

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

No branches or pull requests

2 participants