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

logging_redirect_tqdm not respecting log level of original stream handler #1272

Open
phausamann opened this issue Nov 11, 2021 · 9 comments · May be fixed by #1338
Open

logging_redirect_tqdm not respecting log level of original stream handler #1272

phausamann opened this issue Nov 11, 2021 · 9 comments · May be fixed by #1338
Labels
p2-bug-warning ⚠ Visual output bad p3-enhancement 🔥 Much new such feature

Comments

@phausamann
Copy link

Description

I've run into an issue with logging_redirect_tqdm when the stream handler log level differs from the root logger level.

In my use case, I want to log to a file at debug level and to the console at info level, which is why I need to set the root logger to debug level.

This causes an issue with logging_redirect_tqdm because it uses the root logger level for the redirected handler:

import logging
import sys

from tqdm import trange
from tqdm.contrib.logging import logging_redirect_tqdm

LOG = logging.getLogger(__name__)

if __name__ == '__main__':

    root_logger = logging.getLogger()
    root_logger.setLevel(logging.DEBUG)
    stream_handler = logging.StreamHandler(sys.stdout)
    stream_handler.setLevel(logging.INFO)

    with logging_redirect_tqdm():
        for i in trange(9):
            if i == 4:
                LOG.info("should be printed")
                LOG.debug("should NOT be printed")

Output:

should be printed
should NOT be printed
100%|██████████| 9/9 [00:00<00:00, 28446.67it/s]

Fix

Adding

tqdm_handler.level = orig_handler.level

after this line seems to fix the issue. I'd be happy to open a PR.

Versions

  • tqdm 4.62.2
  • Python 3.8.10
@chengs
Copy link
Contributor

chengs commented Nov 23, 2021

This implementation is intrusive.
image
I think the original handlers should not be updated @casperdcl

@alkatar21
Copy link

@phausamann I have also noticed this, however your example is incorrect as stream_handler is not used there at all.
As example I extended your code as follows (Add formatter to see the difference and register stream_handler):

...
    root_logger = logging.getLogger()
    root_logger.setLevel(logging.DEBUG)
    stream_handler = logging.StreamHandler(sys.stdout)
    stream_handler.setLevel(logging.INFO)
    stream_handler.setFormatter(logging.Formatter(fmt='{levelname:>8}: {message}', style='{')) # Formatter to see the difference
    root_logger.addHandler(stream_handler) # This line is required to see the formatted output (actually use the stream_handler)
...

I would also be happy if the missing line was added to resolve this issue.

@phausamann
Copy link
Author

@alkatar21 you're right, my bad. Here's a modified example that also demonstrates that it works correctly without the context manager:

root_logger = logging.getLogger()
root_logger.setLevel(logging.DEBUG)
stream_handler = logging.StreamHandler(sys.stderr)
stream_handler.setLevel(logging.INFO)
root_logger.addHandler(stream_handler)

with logging_redirect_tqdm():
    for i in trange(9):
        if i == 4:
            LOG.info("should be printed")
            LOG.debug("should NOT be printed, but is")

# works correctly
LOG.info("should be printed")
LOG.debug("should NOT be printed and isn't")

@casperdcl
Copy link
Sponsor Member

@phausamann happy to accept a PR :)

@dot-mike
Copy link

dot-mike commented Jun 5, 2023

Multiple PRs submitted, please review this issue!

@JackZhang2022
Copy link

JackZhang2022 commented Nov 3, 2023

This issue is not resolved in the newest version 4.66.1(Python 3.10.13).
image

So I have to add the following line myself 🙁:

tqdm_handler.setLevel(orig_handler.level)

image

@marcelzwiers
Copy link

If it's really just a oneliner to fix this issue and it's still not fixed (after years of reported issues and multiple unreviewed PRs) then the point has come that I sadly have to ditch tqdm. It's not a minor issue, logging is really important, and tqdm messes it all up

@Hnasar
Copy link

Hnasar commented Apr 18, 2024

here's a wrapper workaround

@contextlib.contextmanager
def logging_redirect_tqdm(
    loggers: list[logging.Logger] | None=None,
    tqdm_class: type[tqdm.std.tqdm]=tqdm.std.tqdm,
    *,
    level: int | None = None,
) -> Iterator[None]:
    """Replaces StreamHandlers with a TQDM handler of the same level."""
    # Fixes https://github.com/tqdm/tqdm/issues/1272
    loggers = loggers or [logging.getLogger()]
    if level is None:
        for logger in loggers:
            for handler in logger.handlers:
                if isinstance(handler, logging.StreamHandler):
                    level = handler.level
                    break

    with tqdm.contrib.logging.logging_redirect_tqdm(loggers, tqdm_class):
        if level is not None:
            for logger in loggers:
                for handler in logger.handlers:
                    if isinstance(handler, tqdm.contrib.logging._TqdmLoggingHandler):
                        handler.setLevel(level)
        yield

@Wyko
Copy link

Wyko commented May 8, 2024

Any update for this yet?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-bug-warning ⚠ Visual output bad p3-enhancement 🔥 Much new such feature
Projects
None yet
9 participants