oemof / DHNx

District heating system optimisation and simulation models
MIT License
27 stars 12 forks source link

Suboptimal usage of the logging module #129

Closed jnettels closed 9 months ago

jnettels commented 1 year ago

Currently, the console output of a script using dhnx looks like this:

INFO:root:CRS of GeoDataFrame converted to EPSG:4647
INFO:root:Connection of buildings completed.
INFO:root:Connection of buildings completed.
INFO:root:Run "boundary" method for finding the building connections
INFO:root:Welding lines... reduced from 504 to 469 lines
INFO:root:Welding lines... reduced from 469 to 456 lines
INFO:root:Welding lines... reduced from 456 to 450 lines

If I redefine the formatting config in my own script like the following, this is simply ignored:

logging.basicConfig(format='%(asctime)s %(levelname)-8s %(message)s', datefmt='%H:%M:%S')

This happens because some files in dhnx use logging.info("..."). Each module should create its own logger, to allow calling logger.info() instead of logging.info(). Otherwise this prevents other scripts importing the module from setting their own formatting and levels per imported module. This is currently inconsistent across the various files.

I have created a branch which attempts to fix the issue. There may be more to the logging moduel that I do not yet know, but at least for me now it works as expected. The lines above would now look like this.

18:55:21 INFO     CRS of GeoDataFrame converted to EPSG:4647
18:55:32 INFO     Connection of buildings completed.
18:55:32 INFO     Connection of buildings completed.
18:55:32 INFO     Run "boundary" method for finding the building connections
18:55:36 INFO     Welding lines... reduced from 504 to 469 lines
18:55:40 INFO     Welding lines... reduced from 469 to 456 lines
18:55:44 INFO     Welding lines... reduced from 456 to 450 lines

Additionally, if I use this in my script, the output is correctly muted:

logging.getLogger('dhnx').setLevel(level='ERROR')

I have pushed this as 8f5e74d6f1e19fec3aa2013fe48d354c7ce46a71 in the branch fix/logger-usage. I have not created a PR because that might be too messy, since that branch also includes the commits from #128 (because I wanted a nice target with all my current work). But the changes are just search and replace for the most part. I could create a dedicated PR after #128 has been merged.