Fix #362: don't reconfigure root logger when importing pyensembl.shell (#363)

* Fix #362: don't reconfigure root logger when importing pyensembl.shell

pyensembl/shell.py called logging.config.fileConfig() at module top-level.
fileConfig defaults to disable_existing_loggers=True, so importing the
module (a) attached pyensembl's console handlers to the root logger and set
its level to INFO, and (b) disabled every logger the host application had
created before the import. A library must not configure the root logger or
disable other loggers as an import side effect.

Following the Python logging HOWTO ("Configuring Logging for a Library"):

- Move the fileConfig() call into a configure_logging() helper that is only
  invoked from the CLI entrypoint (run()), and pass
  disable_existing_loggers=False so it never disables application loggers.
- Attach a NullHandler to the "pyensembl" package logger in __init__.py so
  library use emits nothing by default and never warns about missing
  handlers, while leaving the root logger untouched.

Adds regression tests covering the import side-effects and the CLI logging
setup.

Claude-Session: https://claude.ai/code/session_016pKPSmCAts4cacC3CQ9qsL

* test: restore global logging state in configure_logging test

configure_logging() applies logging.conf, which mutates process-global
logging state (replaces the pyensembl logger's NullHandler with a live
console handler and reconfigures root). Snapshot and restore that state in
test_configure_logging_preserves_existing_loggers so it doesn't leak a
console handler into sibling tests running in the same worker.

Claude-Session: https://claude.ai/code/session_016pKPSmCAts4cacC3CQ9qsL

Author: iskandr

pyensembl/__init__.py

@@ -10,6 +10,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import logging
+
 from .database import Database
 from .download_cache import DownloadCache
 from .ensembl_release import EnsemblRelease, cached_release
@@ -35,6 +37,12 @@
 from .transcript import Transcript
 from .version import __version__
 
+# Per the Python logging HOWTO ("Configuring Logging for a Library"), attach a
+# no-op handler to the package logger so that library usage neither emits log
+# output nor triggers "No handlers could be found" warnings, while leaving the
+# root logger and any application-configured loggers untouched.
+logging.getLogger(__name__).addHandler(logging.NullHandler())
+
 __all__ = [
     "__version__",
     "DownloadCache",

pyensembl/shell.py

@@ -49,10 +49,23 @@
 from .species import Species
 from .version import __version__
 
-logging.config.fileConfig(str(resources.files("pyensembl") / "logging.conf"))
 logger = logging.getLogger(__name__)
 
 
+def configure_logging():
+    """Apply pyensembl's console logging configuration.
+
+    This is only invoked from the command-line entrypoint (``run``) so that
+    merely importing this module never reconfigures the root logger or
+    disables any loggers the host application has already created. See
+    https://github.com/openvax/pyensembl/issues/362.
+    """
+    logging.config.fileConfig(
+        str(resources.files("pyensembl") / "logging.conf"),
+        disable_existing_loggers=False,
+    )
+
+
 parser = argparse.ArgumentParser(usage=__doc__)
 
 parser.add_argument(
@@ -365,6 +378,7 @@ def _w(values, fallback):
 
 
 def run():
+    configure_logging()
     args = parser.parse_args()
     if args.action == "list":
         # TODO: how do we also identify which non-Ensembl genomes are

tests/test_shell.py

@@ -1,5 +1,10 @@
+import logging
+import subprocess
+import sys
+
 from pyensembl.shell import (
     all_combinations_of_ensembl_genomes,
+    configure_logging,
     format_available_species,
     parser,
 )
@@ -66,3 +71,76 @@ def test_format_available_species_collapses_single_release():
     )
     assert "54–54" not in ncbi36_line
     assert "54" in ncbi36_line
+
+
+# Regression test for https://github.com/openvax/pyensembl/issues/362:
+# importing pyensembl / pyensembl.shell must not reconfigure the root logger
+# or disable loggers the host application created before the import. Run in a
+# fresh interpreter because logging state is process-global and modules are
+# only imported once.
+_IMPORT_SIDE_EFFECT_PROBE = """
+import logging
+
+created_before = logging.getLogger("created_before")
+
+import pyensembl
+import pyensembl.shell
+
+# pyensembl's logging.conf attaches a CRITICAL-level StreamHandler to the root
+# logger; it must not be applied merely by importing the package.
+root = logging.getLogger()
+pyensembl_root_handlers = [
+    h for h in root.handlers if getattr(h, "level", None) == logging.CRITICAL
+]
+assert not pyensembl_root_handlers, (
+    "import added pyensembl's console handler to the root logger: %r"
+    % (pyensembl_root_handlers,)
+)
+
+# A logger created before the import must not be disabled
+# (fileConfig(disable_existing_loggers=True) would have disabled it).
+assert created_before.disabled is False, "import disabled a pre-existing logger"
+
+# The package logger should carry a NullHandler so library use neither emits
+# output nor triggers "No handlers could be found" warnings.
+assert any(
+    isinstance(h, logging.NullHandler)
+    for h in logging.getLogger("pyensembl").handlers
+), "pyensembl package logger is missing a NullHandler"
+
+print("ok")
+"""
+
+
+def test_import_does_not_reconfigure_root_logger():
+    result = subprocess.run(
+        [sys.executable, "-c", _IMPORT_SIDE_EFFECT_PROBE],
+        capture_output=True,
+        text=True,
+    )
+    assert result.returncode == 0, result.stderr
+    assert result.stdout.strip().endswith("ok")
+
+
+def test_configure_logging_preserves_existing_loggers():
+    # configure_logging() applies logging.conf, which mutates process-global
+    # logging state (root + pyensembl loggers). Snapshot and restore it so this
+    # test doesn't leak a live console handler into sibling tests.
+    root = logging.getLogger()
+    pyensembl_logger = logging.getLogger("pyensembl")
+    saved_root_handlers = root.handlers[:]
+    saved_root_level = root.level
+    saved_pyensembl_handlers = pyensembl_logger.handlers[:]
+    try:
+        created_before = logging.getLogger("test_configure_logging_preexisting")
+        created_before.disabled = False
+        configure_logging()
+        # The CLI entrypoint applies logging.conf, but with
+        # disable_existing_loggers=False so it leaves other loggers alone.
+        assert created_before.disabled is False
+        # pyensembl's own logger should be wired up to a handler for CLI output.
+        assert pyensembl_logger.handlers
+    finally:
+        root.handlers[:] = saved_root_handlers
+        root.level = saved_root_level
+        pyensembl_logger.handlers[:] = saved_pyensembl_handlers