logger creation removed from __init__.py

[epogrebnyak]

Fixes #11

[danijar]

As mentioned in #11, I think the logger should be created on import so that the user can do logging.getLogger('handout') to change the log level etc. What do you think?

[epogrebnyak]

Regarding your comment for user levels, the user can change it globally by running this line, for example:

logger.setLevel(logging.DEBUG)

Also we actually do not provide any other logging messages than info().

[epogrebnyak]

Well, actually he wouldn't change the log level for that particular logger... need a rethink.

[epogrebnyak]

Here is an idea - one function is logger_config(), and it is called on import, the other fucniton in logger_provide() and it simply returns a proper logger in Handout.

This way the logger is available upon import and configurable by the user, but we keep __init__.py cleaner, more avoiding the hidden state.

[danijar]

This seems a bit too complicated to me. I don't think moving the logic from __init__.py into tools.py makes it less hidden. On the contrary, now it's not clear anymore where and how often the logger is created. Moreover, I don't see the need for the get_logger() function that is just an alias for an existing function in the logging module. In other words, let's just add the one line to __init__.py to check whether there are any handlers already, if you don't mind.

[epogrebnyak]

Well, you can make a one-liner change in __init__.py, but the place is rather hidden, otherwise it would not have taken that much time to find the source of error for #11.

Something happening in __init__.py except for the imports in a bit shaky.

get_logger is not just a sugar for the logging function - it returns a specific logger named 'handout'. Otherwise it is a question how a programmer knows to use this name for the logger, also for configuring it to a different level?

moving the logic from init.py into tools.py makes it less hidden.

It does - there is a dedicated filelogger.py now. If I saw one in codebase, would have saved me a ton of time tracing the duplicate messages error, just as example.

[epogrebnyak]

I can see some preference in keeping the codebase compact without logger.py though, so the decision is up to you.

[danijar]

You raise a valid point. I added the one line check and put a comment in the Handout class to tell contributors where to look for the logging configuration. As you said in your second comment, having a separate file for the logger seems a bit too much to me. There are many parts of the code that could all have separate files but every time we do that the codes gets more spread out and harder to edit. I'm closing the PR since I've added the fix -- thanks a lot for finding the bug in the first place!

[epogrebnyak]

Dev comment is a good idea!