Improved beets/logging.py typing (#6032)

This PR enhances `beets/logging.py` with improved typing and tests:

* `getLogger` now returns the precise logger type (`BeetsLogger` or
`RootLogger`).
* Tests use `pytest` and `parametrize` for more concise and readable
coverage.

Author: semohr

.github/CODEOWNERS

@@ -1,2 +1,5 @@
 # assign the entire repo to the maintainers team
 * @beetbox/maintainers
+
+# Specific ownerships:
+/beets/metadata_plugins.py @semohr

beets/logging.py

@@ -20,6 +20,8 @@
 calls (`debug`, `info`, etc).
 """
 
+from __future__ import annotations
+
 import threading
 from copy import copy
 from logging import (
@@ -32,8 +34,10 @@
     Handler,
     Logger,
     NullHandler,
+    RootLogger,
     StreamHandler,
 )
+from typing import TYPE_CHECKING, Any, Mapping, TypeVar, Union, overload
 
 __all__ = [
     "DEBUG",
@@ -49,8 +53,20 @@
     "getLogger",
 ]
 
+if TYPE_CHECKING:
+    T = TypeVar("T")
+    from types import TracebackType
+
+    # see https://github.com/python/typeshed/blob/main/stdlib/logging/__init__.pyi
+    _SysExcInfoType = Union[
+        tuple[type[BaseException], BaseException, Union[TracebackType, None]],
+        tuple[None, None, None],
+    ]
+    _ExcInfoType = Union[None, bool, _SysExcInfoType, BaseException]
+    _ArgsType = Union[tuple[object, ...], Mapping[str, object]]
+
 
-def logsafe(val):
+def _logsafe(val: T) -> str | T:
     """Coerce `bytes` to `str` to avoid crashes solely due to logging.
 
     This is particularly relevant for bytestring paths. Much of our code
@@ -83,40 +99,45 @@ class StrFormatLogger(Logger):
     """
 
     class _LogMessage:
-        def __init__(self, msg, args, kwargs):
+        def __init__(
+            self,
+            msg: str,
+            args: _ArgsType,
+            kwargs: dict[str, Any],
+        ):
             self.msg = msg
             self.args = args
             self.kwargs = kwargs
 
         def __str__(self):
-            args = [logsafe(a) for a in self.args]
-            kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()}
+            args = [_logsafe(a) for a in self.args]
+            kwargs = {k: _logsafe(v) for (k, v) in self.kwargs.items()}
             return self.msg.format(*args, **kwargs)
 
     def _log(
         self,
-        level,
-        msg,
-        args,
-        exc_info=None,
-        extra=None,
-        stack_info=False,
+        level: int,
+        msg: object,
+        args: _ArgsType,
+        exc_info: _ExcInfoType = None,
+        extra: Mapping[str, Any] | None = None,
+        stack_info: bool = False,
+        stacklevel: int = 1,
         **kwargs,
     ):
         """Log msg.format(*args, **kwargs)"""
-        m = self._LogMessage(msg, args, kwargs)
 
-        stacklevel = kwargs.pop("stacklevel", 1)
-        stacklevel = {"stacklevel": stacklevel}
+        if isinstance(msg, str):
+            msg = self._LogMessage(msg, args, kwargs)
 
         return super()._log(
             level,
-            m,
+            msg,
             (),
             exc_info=exc_info,
             extra=extra,
             stack_info=stack_info,
-            **stacklevel,
+            stacklevel=stacklevel,
         )
 
 
@@ -156,9 +177,12 @@ class BeetsLogger(ThreadLocalLevelLogger, StrFormatLogger):
 my_manager.loggerClass = BeetsLogger
 
 
-# Override the `getLogger` to use our machinery.
-def getLogger(name=None):  # noqa
+@overload
+def getLogger(name: str) -> BeetsLogger: ...
+@overload
+def getLogger(name: None = ...) -> RootLogger: ...
+def getLogger(name=None) -> BeetsLogger | RootLogger:  # noqa: N802
     if name:
-        return my_manager.getLogger(name)
+        return my_manager.getLogger(name)  # type: ignore[return-value]
     else:
         return Logger.root

beetsplug/fetchart.py

@@ -36,10 +36,10 @@
 
 if TYPE_CHECKING:
     from collections.abc import Iterable, Iterator, Sequence
-    from logging import Logger
 
     from beets.importer import ImportSession, ImportTask
     from beets.library import Album, Library
+    from beets.logging import BeetsLogger as Logger
 
 try:
     from bs4 import BeautifulSoup, Tag

beetsplug/lyrics.py

@@ -42,10 +42,9 @@
 from beets.util.config import sanitize_choices
 
 if TYPE_CHECKING:
-    from logging import Logger
-
     from beets.importer import ImportTask
     from beets.library import Item, Library
+    from beets.logging import BeetsLogger as Logger
 
     from ._typing import (
         GeniusAPI,
@@ -186,7 +185,7 @@ def slug(text: str) -> str:
 
 
 class RequestHandler:
-    _log: beets.logging.Logger
+    _log: Logger
 
     def debug(self, message: str, *args) -> None:
         """Log a debug message with the class name."""

docs/changelog.rst

@@ -51,6 +51,12 @@ Other changes:
 - :class:`beets.metadata_plugin.MetadataSourcePlugin`: Remove discogs specific
   disambiguation stripping.
 
+For developers and plugin authors:
+
+- Typing improvements in ``beets/logging.py``: ``getLogger`` now returns
+  ``BeetsLogger`` when called with a name, or ``RootLogger`` when called without
+  a name.
+
 2.4.0 (September 13, 2025)
 --------------------------
 

test/test_logging.py

@@ -3,19 +3,21 @@
 import logging as log
 import sys
 import threading
-import unittest
-from io import StringIO
 from types import ModuleType
 from unittest.mock import patch
 
+import pytest
+
 import beets.logging as blog
 from beets import plugins, ui
 from beets.test import _common, helper
 from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin
 
 
-class LoggingTest(unittest.TestCase):
-    def test_logging_management(self):
+class TestStrFormatLogger:
+    """Tests for the custom str-formatting logger."""
+
+    def test_logger_creation(self):
         l1 = log.getLogger("foo123")
         l2 = blog.getLogger("foo123")
         assert l1 == l2
@@ -35,17 +37,34 @@ def test_logging_management(self):
         l6 = blog.getLogger()
         assert l1 != l6
 
-    def test_str_format_logging(self):
-        logger = blog.getLogger("baz123")
-        stream = StringIO()
-        handler = log.StreamHandler(stream)
-
-        logger.addHandler(handler)
-        logger.propagate = False
-
-        logger.warning("foo {} {bar}", "oof", bar="baz")
-        handler.flush()
-        assert stream.getvalue(), "foo oof baz"
+    @pytest.mark.parametrize(
+        "level", [log.DEBUG, log.INFO, log.WARNING, log.ERROR]
+    )
+    @pytest.mark.parametrize(
+        "msg, args, kwargs, expected",
+        [
+            ("foo {} bar {}", ("oof", "baz"), {}, "foo oof bar baz"),
+            (
+                "foo {bar} baz {foo}",
+                (),
+                {"foo": "oof", "bar": "baz"},
+                "foo baz baz oof",
+            ),
+            ("no args", (), {}, "no args"),
+            ("foo {} bar {baz}", ("oof",), {"baz": "baz"}, "foo oof bar baz"),
+        ],
+    )
+    def test_str_format_logging(
+        self, level, msg, args, kwargs, expected, caplog
+    ):
+        logger = blog.getLogger("test_logger")
+        logger.setLevel(level)
+
+        with caplog.at_level(level, logger="test_logger"):
+            logger.log(level, msg, *args, **kwargs)
+
+        assert caplog.records, "No log records were captured"
+        assert str(caplog.records[0].msg) == expected
 
 
 class DummyModule(ModuleType):