Closed vilhub closed 2 years ago
Hello, @vilhub, and thanks for your pull request.
I understand the removing the call to logging.basicConfig() is sound, but I need to research further on best practices on logging and how to do that in libraries. Your comment does not go into detail in that respect. I did find a very detailed question & answer on Stack Overflow that does go into the particulars, which is leading me in the right direction.
I will be closing this pull request, but once I understand what natto-py needs with respect to logging, I will certainly be making a few changes around that.
Hi @buruzaemon , thanks for the response!
Indeed as the Stack Overflow post explains, the logging configuration should be left to the user as the user of the library should choose where the information is written when an INFO or WARNING log happens for example.
This is resolved simply by removing the basicConfig
line and letting the user take care of configuration, otherwise the user-provided configuration will be ignored in favor of the library's predefined configuration.
This was for the logging handler configuration. Now, another orthogonal dimension is the structuring of the different logger names. It is standard practice to initialise each logger with the module name like logger = logging.getLogger(__name__)
. This ensures creation of a hierarchy of loggers with the dot notation that reflects your library structure.
Link with more information : https://docs.python-guide.org/writing/logging/#logging-in-a-library
Best, Vil
Hello, @vilhub ... sorry it took me so long to decide, but you are absolutely right about the logging configuration issue, and so I am re-opening your original request. Will be merging this in shortly, and then releasing 1.0.1.
Thank you very much for your contribution!
The logger basic configuration should be done by the user application of the library.
Setting the basic configuration inside the library causes the application call to
logging.basicConfig
to be ignored. As a result, whennatto-py
is imported, the default basic configuration is imposed to the whole application.