Skip to content

Commit 1a5de8c

Browse files
eduralphclaude
authored andcommitted
lxml: drop self.uistate.window from module-level ErrorDialog calls
`lxml/lxmlGramplet.py:72,94` reference `self.uistate.window` at module top level — both inside `except ImportError:` branches that guard `import gzip` and `from lxml import etree, objectify`. `self` is undefined at module-load time (the gramplet instance does not exist yet), so when either import actually failed the addon module crashed with: NameError: name 'self' is not defined …instead of showing the intended ErrorDialog. Ruff (F821) flags both: F821 Undefined name `self` --> lxml/lxmlGramplet.py:72:69 --> lxml/lxmlGramplet.py:94:104 PR gramps-project#820's body listed this as one of the "Real bugs": "stray `parent=self.uistate.window` at module level in lxmlGramplet". Drop `parent=self.uistate.window` from both `ErrorDialog` calls. The dialogs render with no parent window — not ideal aesthetically, but at module-load there genuinely is no UI parent to anchor to (the alternative — moving these checks into the gramplet's `__init__` — is a larger restructuring outside this PR's scope). In practice the gzip branch is dead code (gzip is in the Python stdlib and never raises ImportError); the lxml branch is the user-visible win — when a user installs the addon without the lxml package present, they now get the intended "Missing python3 lxml" dialog instead of an opaque NameError that masks the underlying cause. No regression test is added for this fix: - The bug only fires when `from lxml import ...` raises ImportError, which requires either an unusual install or subprocess-isolated sys.modules tampering to simulate. - The natural location for the test would be `lxml/tests/`, but the addon directory name (`lxml`) collides with the third-party `lxml` package — Python's import system treats the latter (regular package, /usr/lib/python3/dist-packages/lxml) as taking precedence over the namespace-package addon directory, so `python3 -m unittest lxml.tests.test_*` cannot reach the addon's tests subtree on any system that has `lxml` installed (which the CI image always does). - Ruff F821 catches future regressions of this exact pattern, which is the same gate that surfaced the bug in the first place. Verified via `ruff check --select=E9,F63,F7,F82 --no-fix --exclude='*.gpr.py' lxml/` (both F821 sites on `self` removed — the remaining F632 sites at lines 400/758 are in-flight via PR gramps-project#837) and `python3 -m py_compile lxml/lxmlGramplet.py`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent eab6201 commit 1a5de8c

1 file changed

Lines changed: 2 additions & 2 deletions

File tree

lxml/lxmlGramplet.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
GZIP_OK = True
7070
except ImportError:
7171
GZIP_OK = False
72-
ErrorDialog(_('Where is gzip?'), _('"gzip" is missing'), parent=self.uistate.window)
72+
ErrorDialog(_('Where is gzip?'), _('"gzip" is missing'))
7373
LOG.error('No gzip')
7474

7575
#-------------------------------------------------------------------------
@@ -91,7 +91,7 @@
9191
LIBXSLT_VERSION = etree.LIBXSLT_VERSION
9292
except ImportError:
9393
LXML_OK = False
94-
ErrorDialog(_('Missing python3 lxml'), _('Please, try to install "python3 lxml" package.'), parent=self.uistate.window)
94+
ErrorDialog(_('Missing python3 lxml'), _('Please, try to install "python3 lxml" package.'))
9595
LOG.debug('No lxml')
9696

9797
#-------------------------------------------------------------------------

0 commit comments

Comments
 (0)