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

Set only default level for the sox logger #65

Merged
merged 2 commits into from
Sep 30, 2020
Merged

Conversation

machste
Copy link
Contributor

@machste machste commented Jan 8, 2018

In the following example, I'd expect log messages from the example logger and only warnings or error messages from the sox logger.

import logging
import sox

# Setup logging
logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger()

# Play something.mp3
player = sox.Transformer()
logger.debug("Play ...")
player.preview("something.mp3")
logger.debug("Done.")

Although, the default level of the sox logger is set to logging.WARNING , I get the info messages anyway:

DEBUG:root:Play ...
INFO:root:Executing: play --no-show-progress -D -V2 something.mp3
INFO:root:<open file '<fdopen>', mode 'rb' at 0xb719f548>
INFO:root:Play returned with error code 2
DEBUG:root:Done.

In my opinion the log level should be set to the sox logger rather than to the console StreamHandler which gets overwritten by the logger.basicConfig anyway.

Furthermore, I don't see the need of setting up a log handler here. This should be done by the user of the library.

However, I really appreciate the work, which has been put into this project. Thank you! :-)

Cheers,
Stefan

The sox logger shall only set its default log level rather than
setup log handler.
@coveralls
Copy link

coveralls commented Jan 8, 2018

Coverage Status

Coverage decreased (-0.001%) to 99.103% when pulling e450a19 on machste:master into 4de1bcf on rabitt:master.

@rabitt
Copy link
Collaborator

rabitt commented Sep 30, 2020

2 years later... merging 😬

Thanks @machste !

@rabitt rabitt merged commit 2c3c982 into marl:master Sep 30, 2020
@machste
Copy link
Contributor Author

machste commented Sep 30, 2020

Better late than never! ;-)
You're welcome!

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.

None yet

3 participants