at-python-template icon indicating copy to clipboard operation
at-python-template copied to clipboard

Add logging example code for when create_cli==no

Open MaxPensel opened this issue 4 years ago • 4 comments

Extended files:

{{cookiecutter.project_slug}}/src/{{cookiecutter.module_name}}/main.py
{{cookiecutter.project_slug}}/notebooks/example.ipynb

Affected option: config_file

Chosing hocon option creates the following code in main.py:

import logging
from test_hocon import util

logger = logging.getLogger("test_hocon")


def main():
    config = util.load_config()
    util.logging_setup(config)

    logger.info("Looks like you're all set up. Let's get going!")

    # TODO your journey starts here
    print("hello :)")


if __name__ == "__main__":
    main()

and adds this cell content to example.ipynb:

import logging
from test_hocon import util

logger = logging.getLogger("test_hocon")

config = util.load_config()
util.logging_setup(config)

logger.info("Use this setup to start logging in notebooks or to "+
            "get the correct logging format in your test_hocon module")

Chosing yaml option creates the following code in main.py:

import os
import logging
from test_yaml import util

logger = logging.getLogger("test_yaml")


def main():
    config = util.load_config(os.path.join(os.path.abspath(__file__),
                                           *[os.pardir]*3,
                                           "config",
                                           "config.yml"))
    util.logging_setup(config)

    logger.info("Looks like you're all set up. Let's get going!")

    # TODO your journey starts here
    print("hello :)")


if __name__ == "__main__":
    main()

and adds this cell content to example.ipynb:

import os
import logging
from test_yaml import util

logger = logging.getLogger("test_yaml")

config = util.load_config(os.path.join(os.path.abspath(os.pardir),
                                       "config",
                                       "config.yml"))
util.logging_setup(config)

logger.info("Use this setup to start logging in notebooks or to "+
            "get the correct logging format in your test_yaml module")

The path construction to the config.yml may not be ideal, but could be fixed when addressing issue #38 .

MaxPensel avatar Feb 18 '21 10:02 MaxPensel

In reference to the issue mentioned in PR #26, should python files containing cookiecutter syntax be excluded in .pre-commit-config.yaml? Otherwise the check-ast pre-commit hook will always fail parsing .py files as correct python syntax.

MaxPensel avatar Feb 18 '21 13:02 MaxPensel

I adjusted main.py because I had a few issues with it: discovery of the config file was too complex, just providing a static path should be enough to get you started, and hocon or having no config was not a well-supported case.

I'm still not sure about this PR, because we're adding a lot of complexity here that many of our users might not care for.

If you want to continue with this, can you adjust the notebook code in a similar fashion?

klamann avatar Feb 26 '21 15:02 klamann

ok, now I'm also not happy with the static config path, because it probably won't work by default for most users (e.g. if you launch it in PyCharm, it will try to find the config relative to main.py and not the project root folder).

well... I don't know. If we have the CLI, we can let the user define the path to the config, but in this case, I don't think we're setting a good example by defining a path in the code where the config has to be.

klamann avatar Feb 26 '21 15:02 klamann

Yeah, that is the reason why I opted to build the relative path from main.py to the config using os.

I agree that fixing a path in the library is not good design, I thought of it more as an example, so that people would get a hint at how to make use of logging. In that line of reasoning, it would still make sense to have a static path in the notebook, because that is not part of the python package that is being built, and notebooks are always executed from where they are, so the relative path from example.ipynb to the config should work in any setup.

I also agree that complexity should be kept low, but at the same time we want to encourage good design, which includes logging instead of printing wherever possible. My first experience with the template was just that it didn't paint a clear picture of how to utilize the logger correctly.

MaxPensel avatar Feb 27 '21 15:02 MaxPensel