Repository navigation
Scope the log level set by verbose to the call - #1
Closed
sappelhoff wants to merge 2 commits into
Closed
sappelhoff wants to merge 2 commits into
sappelhoff wants to merge 2 commits into
Conversation
`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
force-pushed
the
fix-verbose-log-level
branch
from
August 8, 2026 10:11
d125ac3 to
6ac8296
Compare
Owner
Author
|
Submitted upstream as xdf-modules#175. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:So one call reconfigures logging for the rest of the process. Two consequences:
pyxdfto its likingloses that configuration to any library that happens to call
load_xdf(verbose=False)—silently, and from code the application does not own.
verbose=None. The docstring saysNone"willuse root logger level", which holds only while the logger's own level is
NOTSET. Afterany earlier call that passed
verbose, the logger has a level of its own and no longerinherits, so every later
verbose=Nonecall silently keeps the old setting.The fix restores the level when the call returns, via a small decorator so the body of
load_xdfis untouched and its signature is preserved throughfunctools.wraps:verbosekeeps working exactly as documented during the call; it just no longer leaks outof 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 soload_xdfraises after the level has been set:verbosein(None, True, False)verbose=Truecallverbosestill decides whether the "Importing XDF file" record is emitted, so the fixcannot be mistaken for deleting the feature
Against the current release these fail as expected —
assert 10 == 40, the logger left atDEBUGwhere the application had setERROR.pytest test/: 366 passed, 13 skipped, 1 xfailed.ruff checkandruff format --checkclean.
Notes
main, which is one commit behindxdf-modules/main(adependabot actions bump, no code). Rebase before opening upstream.
#175as the upstream PR number, a guess from#174being thelatest. Needs confirming when the upstream PR exists.
load_xdfsets the level onpyxdf.pyxdfrather than thepyxdfparent, 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.