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

Used named logger #106

Closed
wants to merge 1 commit into from
Closed

Used named logger #106

wants to merge 1 commit into from

Conversation

flopp
Copy link
Contributor

@flopp flopp commented Jan 2, 2018

Use a named loggers in all modules of gpxpy (log = mod_logging.getLogger(__name__)) -- as recommended by https://docs.python.org/3/library/logging.html
This enables applications to configure gpxpy's logger separately (e.g. logging.getLogger('gpxpy').setLevel(logging.CRITICAL)...).

@coveralls
Copy link

coveralls commented Jan 2, 2018

Coverage Status

Coverage increased (+0.02%) to 84.877% when pulling 2c12403 on flopp:named-logger into e28f59c on tkrajina:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 84.877% when pulling 2c12403 on flopp:named-logger into e28f59c on tkrajina:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 84.877% when pulling 2c12403 on flopp:named-logger into e28f59c on tkrajina:master.

@nawagers
Copy link
Contributor

nawagers commented Jan 3, 2018

What if log = mod_logging.getLogger(__name__) is placed inside the relevant function/class instead?

When the call is placed at the module level it will create a new logger during import. If the dev using the library then loads a logger config from a file logging.config.fileConfig(), it will disable previous loggers by default. The call to getLogger() doesn't create a new instance each time, after the first call it just returns the previously instantiated object. https://docs.python.org/3.6/library/logging.config.html#logging.config.fileConfig

@flopp
Copy link
Contributor Author

flopp commented Jan 3, 2018

I think that would be possible.

BTW, my pull request is based on the logging pattern found in other popular libraries, e.g. https://github.com/shazow/urllib3/blob/master/urllib3/response.py

@ghost
Copy link

ghost commented Jan 4, 2018

I think that initializing loggers in every single function or class of a module can really clutter up the code. The red warning box in the python 3.6 documentation makes it pretty clear that the default of disable_existing_loggers is only 'true' for legacy reasons and that likely most people actually want it to be 'false'. I guess the reason is precisely to have module level loggers to behave as expected.

However since gpxpy does not have that much logging, class/function level loggers wouldn't clutter the code too much here. So in this case one could go either way.

edit: Warning box at https://docs.python.org/3.6/howto/logging.html , about 2/3 down the page

@tkrajina
Copy link
Owner

Merged in dev at the moment, will be in master for the next release.

@tkrajina tkrajina closed this Feb 26, 2018
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 this pull request may close these issues.

4 participants