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

Suboptimal usage of the logging module #129

Closed
jnettels opened this issue Jun 16, 2023 · 0 comments · Fixed by #132
Closed

Suboptimal usage of the logging module #129

jnettels opened this issue Jun 16, 2023 · 0 comments · Fixed by #132
Assignees

Comments

@jnettels
Copy link
Contributor

jnettels commented Jun 16, 2023

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 8f5e74d 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.

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

Successfully merging a pull request may close this issue.

1 participant