Defer more imports for even faster CLI#17
Open
hugovk wants to merge 1 commit intohenryiii:henryiii/chore/fasthelpfrom
Open
Defer more imports for even faster CLI#17hugovk wants to merge 1 commit intohenryiii:henryiii/chore/fasthelpfrom
hugovk wants to merge 1 commit intohenryiii:henryiii/chore/fasthelpfrom
Conversation
Reviewer's GuideOptimizes CLI startup by deferring many typing- and collections-related imports to TYPE_CHECKING-only blocks, enabling use of from future import annotations and tightening lazy module lists across the codebase while updating annotations to use native PEP 604/PEP 585 syntax. Flow diagram for CLI startup with deferred imports and TYPE_CHECKING guardsflowchart TD
start["Start: python -m cibuildwheel --help"]
import_main["Import cibuildwheel.__main__"]
load_lazy_list["Initialize __lazy_modules__ lists in imported modules"]
exec_main["Parse arguments and prepare to show help"]
check_type_checking["Evaluate TYPE_CHECKING = False in modules"]
skip_tc["Skip TYPE_CHECKING blocks (no typing or collections.abc imports)"]
need_platforms["Access platform specific help and defaults"]
import_platforms["Import cibuildwheel.platforms and selected platform modules"]
lazy_not_triggered["Most __lazy_modules__ entries not imported during --help"]
show_help["Render and print help text"]
end_node["Exit"]
start --> import_main --> load_lazy_list --> exec_main
exec_main --> check_type_checking --> skip_tc
exec_main --> need_platforms --> import_platforms
import_platforms --> lazy_not_triggered
skip_tc --> lazy_not_triggered
lazy_not_triggered --> show_help --> end_node
subgraph deferred_imports["Deferred imports"]
typing_block["typing, collections.abc, Path, Protocol, Self, etc. under if TYPE_CHECKING"]
heavy_runtime["bashlex, tomllib, dependency_groups, filelock, elftools, etc. via __lazy_modules__"]
end
skip_tc --> typing_block
lazy_not_triggered --> heavy_runtime
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Defining
TYPE_CHECKING = Falseinstead of importingTYPE_CHECKINGfromtypingmeans type checkers will treat those guarded blocks as regular runtime code (or ignore the guard entirely); consider reverting tofrom typing import TYPE_CHECKINGso tools correctly understandif TYPE_CHECKING:blocks even if that costs a small import. - In places where
TYPE_CHECKINGis used for control flow (e.g.unit_test/get_platform_test.pyskipping on non-Windows), replacingtyping.TYPE_CHECKINGwith a localTYPE_CHECKING = Falsechanges the semantics under type checkers; keep using the standardtyping.TYPE_CHECKINGconstant when you need environment-sensitive logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Defining `TYPE_CHECKING = False` instead of importing `TYPE_CHECKING` from `typing` means type checkers will treat those guarded blocks as regular runtime code (or ignore the guard entirely); consider reverting to `from typing import TYPE_CHECKING` so tools correctly understand `if TYPE_CHECKING:` blocks even if that costs a small import.
- In places where `TYPE_CHECKING` is used for control flow (e.g. `unit_test/get_platform_test.py` skipping on non-Windows), replacing `typing.TYPE_CHECKING` with a local `TYPE_CHECKING = False` changes the semantics under type checkers; keep using the standard `typing.TYPE_CHECKING` constant when you need environment-sensitive logic.
## Individual Comments
### Comment 1
<location path="cibuildwheel/architecture.py" line_range="23" />
<code_context>
from cibuildwheel.util.helpers import strtobool
from cibuildwheel.util.resources import read_all_configs
+TYPE_CHECKING = False
+if TYPE_CHECKING:
+ from collections.abc import Generator, Iterable, Sequence
</code_context>
<issue_to_address>
**issue:** Use `typing.TYPE_CHECKING` instead of a plain `TYPE_CHECKING = False` so type-checkers can see the guarded imports.
Overriding `TYPE_CHECKING` as a plain boolean makes the guarded block invisible to static type checkers as well as at runtime. Most tools special‑case `typing.TYPE_CHECKING` only, so the imports inside these blocks (e.g. `PlatformName`, `Set`, `Literal`, etc.) won’t be recognized and related annotations may degrade to `Any`. This pattern appears in multiple modules in this change. Please restore `from typing import TYPE_CHECKING` and use `if TYPE_CHECKING:` without redefining it so imports remain type‑checker visible but don’t run at runtime.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
|
Lazy imports should work on deferred annotations, though, and those are present in 3.15+ (3.14+). So the TYPE_CHECKING trick is useful, and any time you can move something like TypeVar into a TYPE_CHECKING block should be helpful, but it shouldn't otherwise speed things up (unless deferred annotations are triggering lazy imports, which I |
Author
|
I think it's actually mostly from the Whereas without it they'll be evaluated, triggering more lazy imports to be reified. |
Owner
|
They are not evaluated in 3.14+, and I checked to make sure they don't trigger a lazy import (they don't). They are a bit slower to set up than just leaving them as strings, though, so it could just be that savings. |
Owner
|
This is what I tried: The pathlib one wouldn't pass if they got triggered. |
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
2 participants
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.
This builds on and updates pypa#2797.
We can avoid a lot more imports by using
TYPE_CHECKING=Falseinstead of importing it fromtyping, and move lots of othertypingimports underif TYPE_CHECKING:. Also__future__ import annotationslets us move many more under this guard.This PR is 1.4x faster than pypa#2797 and 4x faster than upstream
main. Using Python 3.15.0a7 from official macOS installer.Summary by Sourcery
Defer type-related imports and enable postponed annotations across the codebase to reduce startup overhead and speed up the CLI.
Enhancements:
from __future__ import annotationsand guard most typing and collections.abc imports with a TYPE_CHECKING flag to avoid importing them at runtime.