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

Distfit modifies the root logger globally #51

Open
JulianFerry opened this issue Feb 26, 2025 · 3 comments
Open

Distfit modifies the root logger globally #51

JulianFerry opened this issue Feb 26, 2025 · 3 comments

Comments

@JulianFerry
Copy link

JulianFerry commented Feb 26, 2025

Description

Importing distfit modifies all logs in our project, this does not happen with any other packages. This happens because distfit modifies the root logger globally: https://github.com/erdogant/distfit/blob/master/distfit/distfit.py#L26

Since distfit is a library it should at least use a local logger with logger = logging.getLogger(__name__), and ideally it should not be configuring logs for its downstream applications.

Minimal reproducible example

import logging

logging.basicConfig(
    format="%(asctime)s %(name)-12s %(levelname)-8s %(message)s",
    datefmt="%Y-%m-%d %H:%M:%S",
    level=logging.INFO,
)

logger = logging.getLogger(__name__)

logger.info("This uses the basic configuration")

from distfit import distfit

logger.info("This is modified by distfit")

Output:

2025-02-26 17:24:55 __main__     INFO     This uses the configuration
[distfit] >INFO> This is modified by distfit

Expected behaviour

Importing distfit should not modify existing logger

@JulianFerry
Copy link
Author

JulianFerry commented Feb 26, 2025

The workaround I have for this at the moment is:

from contextlib import contextmanager

@contextmanager
def preserve_root_logger():
    """Preserve the root logger when executing code.

    This is to stop distlib from modifying the root logger:
    https://github.com/erdogant/distfit/issues/51
    """
    # Save original logger state
    root_logger = logging.getLogger()
    original_handlers = root_logger.handlers[:]
    original_level = root_logger.level

    try:
        yield  # Allow code to run inside the context
    finally:
        # Restore original handlers
        root_logger.handlers.clear()
        for handler in original_handlers:
            root_logger.addHandler(handler)
        root_logger.setLevel(original_level)

with preserve_root_logger():
    import distfit

@erdogant
Copy link
Owner

erdogant commented Mar 2, 2025

Thank you for the feedback.
When I take over your example, the logger name also seems not to change when using another library.
So I created a few adjustments on your example:

logger = logging.getLogger('')
[logger.removeHandler(handler) for handler in logger.handlers[:]]
logging.basicConfig(
    format="%(asctime)s [%(name)-12s] >%(levelname)-8s %(message)s",
    datefmt="%d-%m-%y %H:%M:%S",
    level=logging.INFO)
logger = logging.getLogger(__name__)

I first set the logger to <empty>, then I remove previous handles, and finally set it to the new __name__ again.
Would this scenario also work in your case? In my examples, this works now.

@erdogant
Copy link
Owner

erdogant commented Mar 2, 2025

pip install git+https://github.com/erdogant/distfit

It should be version >= 1.8.2

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

No branches or pull requests

2 participants