chore: use a custom lazy-exclude-modules list#18
Conversation
Reviewer's GuideIntroduces an automated workflow (via nox and pre-commit) to generate and enforce a custom flake8-lazy exclude-modules list, and updates the package’s lazy_modules definitions and configuration accordingly. Sequence diagram for the new lazy modules exclusion update workflowsequenceDiagram
actor Dev
participant pre_commit
participant nox
participant update_lazy_modules
participant cibuildwheel
participant flake8_lazy
participant ruff_prek
Dev->>pre_commit: git commit
pre_commit->>nox: run hook lazy
nox->>update_lazy_modules: run session update_lazy_modules
update_lazy_modules->>cibuildwheel: session.run python -v -m cibuildwheel --help
update_lazy_modules-->>update_lazy_modules: parse lazy modules from output
update_lazy_modules-->>update_lazy_modules: update .flake8 lazy-exclude-modules
alt config_changed or --force
update_lazy_modules->>flake8_lazy: session.run uvx flake8-lazy --apply set --lazy-exclude-modules
opt --force
update_lazy_modules->>ruff_prek: session.run uvx prek run --all-files ruff-check ruff-format
end
end
alt config_changed
update_lazy_modules->>nox: session.error lazy modules exclude list changed
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
update_lazy_modulessession rewrites.flake8with only thelazy-exclude-modulesoption, which will discard any existing flake8 configuration; consider merging into an existing config (or a dedicated include file/section) rather than replacing it entirely. - The
update_lazy_moduleslogic depends on parsingpython -voutput and the# destroyprefix, which is fairly brittle against CPython changes; adding a sanity check (e.g. failing if no or very few modules are detected) or isolating this parsing into a helper with clear assumptions would make it safer to maintain. - The pre-commit
lazyhook always runsnox -s update_lazy_modules, which will be relatively heavy and may fail on environments without Python 3.15; you might want to guard the session against missing interpreters or make the hook opt-in for contributors who need to update the lazy module list.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `update_lazy_modules` session rewrites `.flake8` with only the `lazy-exclude-modules` option, which will discard any existing flake8 configuration; consider merging into an existing config (or a dedicated include file/section) rather than replacing it entirely.
- The `update_lazy_modules` logic depends on parsing `python -v` output and the `# destroy ` prefix, which is fairly brittle against CPython changes; adding a sanity check (e.g. failing if no or very few modules are detected) or isolating this parsing into a helper with clear assumptions would make it safer to maintain.
- The pre-commit `lazy` hook always runs `nox -s update_lazy_modules`, which will be relatively heavy and may fail on environments without Python 3.15; you might want to guard the session against missing interpreters or make the hook opt-in for contributors who need to update the lazy module list.
## Individual Comments
### Comment 1
<location path="noxfile.py" line_range="208" />
<code_context>
session.run("python", "-m", "build")
+@nox.session(default=False, reuse_venv=True, python="3.15")
+def update_lazy_modules(session: nox.Session) -> None:
+ """
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `python="3.15"` may make this session unusable on many environments until 3.15 is widely available.
Hard-coding `python="3.15"` will cause this session to fail on machines without that interpreter, which will be common for some time. Unless you truly require 3.15-only behavior, consider targeting a more widely available version (e.g., your minimum supported or CI default) or omitting the `python` argument so the session can run with the local default interpreter.
</issue_to_address>
### Comment 2
<location path="noxfile.py" line_range="230-232" />
<code_context>
+ "posixpath", # POSIX (Linux/macOS)
+ }
+ for line in output.splitlines():
+ destroy = "# destroy "
+ if line.startswith(destroy):
+ name = line[len(destroy) :]
+ extern_private = name.startswith("_") or ("._" in name and "cibuildwheel" not in name)
+ if extern_private or name in platform_specific:
</code_context>
<issue_to_address>
**issue (bug_risk):** Parsing `python -v` output via `"# destroy "` is tightly coupled to CPython’s current verbose import format.
This relies on CPython’s verbose import debug format, which is not a stable API and may change between versions, potentially yielding an incorrect lazy-module list without obvious failure. Please consider adding a version guard, validating that we actually collect modules (and failing clearly if not), or encapsulating this parsing so it’s easier to update if the format changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| session.run("python", "-m", "build") | ||
|
|
||
|
|
||
| @nox.session(default=False, reuse_venv=True, python="3.15") |
There was a problem hiding this comment.
issue (bug_risk): Using python="3.15" may make this session unusable on many environments until 3.15 is widely available.
Hard-coding python="3.15" will cause this session to fail on machines without that interpreter, which will be common for some time. Unless you truly require 3.15-only behavior, consider targeting a more widely available version (e.g., your minimum supported or CI default) or omitting the python argument so the session can run with the local default interpreter.
| destroy = "# destroy " | ||
| if line.startswith(destroy): | ||
| name = line[len(destroy) :] |
There was a problem hiding this comment.
issue (bug_risk): Parsing python -v output via "# destroy " is tightly coupled to CPython’s current verbose import format.
This relies on CPython’s verbose import debug format, which is not a stable API and may change between versions, potentially yielding an incorrect lazy-module list without obvious failure. Please consider adding a version guard, validating that we actually collect modules (and failing clearly if not), or encapsulating this parsing so it’s easier to update if the format changes.
899fb7a to
8ff54de
Compare
a22493a to
a658c51
Compare
8ff54de to
cd79b86
Compare
the list can be updated using `nox -s update_lazy_modules`
as mentioned in pypa#2797 (review), if it's deemed too noisy with the default configuration, maybe at least a part of this could be re-used.
Summary by Sourcery
Introduce tooling to automatically maintain a custom lazy-import exclusion list and align existing lazy module declarations with it.
Enhancements:
Build: