Implement loggers

[barneydobson]

Description

Implement loggers for only the existing print statements, this behaviour should probably be expanded: #71

Fixes #47

I went with @cheginit 's suggestion because it just seemed a bit easier... and my office is way too hot to watch a youtube video on logging... (blame the civil engineering central heating..)

[cheginit]

I recommend to call logger = utils.get_logger() inside functions that you want to use it instead of creating the logger object globally. This ensures that the verbose env variable gets checked before calling the function, so can disable/enable it on the fly.

[barneydobson]

I recommend to call logger = utils.get_logger() inside functions that you want to use it instead of creating the logger object globally. This ensures that the verbose env variable gets checked before calling the function, so can disable/enable it on the fly.

@cheginit if I call logger.disable('swmmanywhere') it will have the same effect and disable globally (or at least as far as I was able to test) - this seems easier than instantiating a new logger inside each function where it is needed?

[dalonsoa]

It's a good excuse - and loguro is nice, anyway.

It looks good to me. Contrary to @cheginit , I'd go with the global variable approach. Loggers are one of those things that are better done with globals: you set it once and you don't touch it while the program is running.

What I would do, however, is to put the logger in its dedicated module logging as chances are that, in the future, you want to expand and deal with that logging on its own.

[cheginit]

Move this call to each function that requires logging, as it leads to unpredictable behavior when changing the verbosity using SWMMANYWHERE_VERBOSE env var. This happens because when you call it here in the utils module, you read the SWMMANYWHERE_VERBOSE variable only when the package is being loaded. So, if a user sets SWMMANYWHERE_VERBOSE after loading the package, it won't work.

Essentially, this works:

import os

os.environ["SWMMANYWHERE_VERBOSE"] = "true"

import swmmanywhere

but this doesn't work as expected:

import swmmanywhere
import os

os.environ["SWMMANYWHERE_VERBOSE"] = "true"

But if you call it within functions, it will always work correctly, regardless of the import order.

[barneydobson]

OK I tidied it all up a bit and added some tests (for these - see

SWMManywhere/tests/test_logging.py

Line 32 in a8c5d91

def test_logger_disable():

and

SWMManywhere/tests/test_logging.py

Line 45 in a8c5d91

def test_logger_reimport():

).

Simply importing logger and setting logger.enable('swmmanywhere') or logger.disable('swmmanywhere') seems to work fine globally regardless of when the import happens - if you can write a test that breaks it then I'll reconsider ;)

[cheginit]

I find it easier and more flexible to use an env variable to control the verbosity rather than importing a function. You can set it from the command-line, or anywhere in code, without having to know the logger syntax.

[barneydobson]

OK this is a good argument for global variables!

So I have added a filter that enables control of verbosity through globals, but also continues use of a global logger.

@cheginit and @dalonsoa - is this actually a good idea or am I opening up for a world of hurt?

Otherwise I will just go with @cheginit 's initial suggestion and import the logger locally each time it is used, still controlling it through globals.

[dalonsoa]

It looks good to me. The dynamic filter seems to enable the best of both worlds.

[cheginit]

Yeah, LGTM.

[cheginit]

This can be return os.getenv("SWMMANYWHERE_VERBOSE", "false").lower() == "true"