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

Drop logging configuration settings #26

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Drop logging configuration settings #26

merged 1 commit into from
Feb 20, 2025

Conversation

DavidStirling
Copy link
Member

This PR addresses two problems:

  • omero2pandas currently applies configuration to the root logger, which may interfere with configuration performed by end users. It should probably be left up to the consuming application to do this.
  • The applied configuration does not specify a log level, meaning that it'll default to only showing WARNING and above. This can lead to a counterintuitive scenario where the user expects to see INFO messages, but omero2pandas has suppressed them.

I think the solution here is to not be opinionated about logging format at all. We'll let omero2pandas use whatever was configured by the application calling it. Since we (currently?) don't offer a CLI we shouldn't need to touch the root logger ourselves.

Testing

Make a simple test script:

import logging
import omero2pandas

logger = logging.getLogger(__name__)
logging.basicConfig(level=logging.DEBUG, format='%(levelname)-7s %(message)s')

logger.info("Foo")
logger.warning("Bar")

When run this produces:

2025-02-19 15:04:52,249 WARNING [        __main__] Bar

This happens as omero2pandas applied it's logging settings.

Comment out import omero2pandas and you should get:

INFO    Foo
WARNING Bar

With this PR you should always get the latter result.

Copy link
Member

@sbesson sbesson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @DavidStirling. I completely agree with the change here. As a library, omero2pandas should not be opinionated on setting up the configuration of the logger and leave this responsibility to the caller script or application.

If we ever introduce an utility available e.g. as an OMERO CLI extension or a single entrypoint, this can set the configuration (and control it via options).

@sbesson sbesson merged commit 758acc3 into main Feb 20, 2025
1 check passed
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.

2 participants