Don't call logging.basicConfig() in __init__.py
#659
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #659 +/- ##
==========================================
- Coverage 81.48% 81.48% -0.01%
==========================================
Files 41 41
Lines 3900 3899 -1
==========================================
- Hits 3178 3177 -1
Misses 722 722
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
| logging.basicConfig( | ||
| format="%(levelname)s: %(message)s", | ||
| level=getattr(logging, os.environ.get("HEUDICONV_LOG_LEVEL", "INFO")), | ||
| ) |
There was a problem hiding this comment.
Please add it then to https://github.com/nipy/heudiconv/blob/master/heudiconv/cli/run.py
There was a problem hiding this comment.
Should I also add it to heudiconv/cli/monitor.py?
There was a problem hiding this comment.
sure so we don't wonder later on. But then may be place it into some function config_logging or alike so we do not duplicate definitions.
There was a problem hiding this comment.
@yarikoptic For some reason, monitor.py already configures the logger for that module to use a different log format than the rest of heudiconv. Should this be retained?
There was a problem hiding this comment.
heh, ok then, let's just proceed for now. Not yet sure if we ever get back to monitor to make it actually used.
|
Thank you! |
|
🚀 PR was released in |
Calling
logging.basicConfig()when your library is imported is incredibly rude, as it infringes upon the importing code's ability to configure the logging itself. The only time (if any) theheudiconvpackage should calllogging.basicConfig()is in themain()function for a command-line entry point.See also https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library.