Skip to content

Scope the log level set by verbose to the call - #1

Closed
sappelhoff wants to merge 2 commits into
mainfrom
fix-verbose-log-level
Closed

sappelhoff wants to merge 2 commits into
mainfrom
fix-verbose-log-level

Conversation

@sappelhoff

Copy link
Copy Markdown
Owner

Draft for review on the fork before it goes to xdf-modules.

load_xdf(verbose=...) sets the level on the module logger and never restores it:

if verbose is not None:
    logger.setLevel(logging.DEBUG if verbose else logging.WARNING)

So one call reconfigures logging for the rest of the process. Two consequences:

  1. It overrides the application. A program that had configured pyxdf to its liking
    loses that configuration to any library that happens to call load_xdf(verbose=False) —
    silently, and from code the application does not own.
  2. It breaks the documented meaning of verbose=None. The docstring says None "will
    use root logger level", which holds only while the logger's own level is NOTSET. After
    any earlier call that passed verbose, the logger has a level of its own and no longer
    inherits, so every later verbose=None call silently keeps the old setting.

The fix restores the level when the call returns, via a small decorator so the body of
load_xdf is untouched and its signature is preserved through functools.wraps:

@_scoped_log_level
def load_xdf(...):

verbose keeps working exactly as documented during the call; it just no longer leaks out
of it. Restoration happens on the error path too, which is where a leak is most likely to go
unnoticed.

Tests

test/test_logging.py, 7 cases, no data files needed — they use a nonexistent path so
load_xdf raises after the level has been set:

  • the level is unchanged after the call for verbose in (None, True, False)
  • a level the application set beforehand survives a verbose=True call
  • verbose still decides whether the "Importing XDF file" record is emitted, so the fix
    cannot be mistaken for deleting the feature

Against the current release these fail as expected — assert 10 == 40, the logger left at
DEBUG where the application had set ERROR.

pytest test/: 366 passed, 13 skipped, 1 xfailed. ruff check and ruff format --check
clean.

Notes

  • Branched from this fork's main, which is one commit behind xdf-modules/main (a
    dependabot actions bump, no code). Rebase before opening upstream.
  • The changelog entry cites #175 as the upstream PR number, a guess from #174 being the
    latest. Needs confirming when the upstream PR exists.
  • Deliberately left alone: load_xdf sets the level on pyxdf.pyxdf rather than the pyxdf
    parent, so while a call is in flight it still overrides a level set on the parent
    namespace. Now that the override is scoped to the call, that is a much smaller wart, and
    changing which logger is addressed would be a behaviour change rather than a fix.

dependabot Bot and others added 2 commits June 24, 2026 06:06
`load_xdf(verbose=...)` set the level on the module logger and never restored
it, so a single call reconfigured logging for the rest of the process. It also
broke the documented meaning of `verbose=None` ("use root logger level") for
every later call: once the logger has a level of its own, it no longer inherits.

An application that had configured `pyxdf` to its liking would silently lose
that configuration to any library that happened to pass `verbose`.
@sappelhoff
sappelhoff force-pushed the fix-verbose-log-level branch from d125ac3 to 6ac8296 Compare August 8, 2026 10:11
@sappelhoff

Copy link
Copy Markdown
Owner Author

Submitted upstream as xdf-modules#175.

@sappelhoff sappelhoff closed this Aug 8, 2026
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.

1 participant