Skip to content

Don't exclude imports in the else branch of TYPE_CHECKING guards - #324

Open
pylaterreur wants to merge 1 commit into
python-grimp:mainfrom
pylaterreur:fix-type-checking-else
Open

pylaterreur wants to merge 1 commit into
python-grimp:mainfrom
pylaterreur:fix-type-checking-else

Conversation

@pylaterreur

Copy link
Copy Markdown

Fixes #323.

With exclude_type_checking_imports=True, the imports in the elif/else branches of an if TYPE_CHECKING: statement were excluded along with the guarded ones, even though those branches run at runtime. Contracts that such imports break were therefore kept. For example, build_graph("filelock", exclude_type_checking_imports=True) has no import from filelock to filelock._read_write or filelock._async_read_write, which filelock/__init__.py imports in such an else branch.

The import parser now only marks the branches guarded by TYPE_CHECKING as type checking only:

  • else branches, and elif branches with other conditions, are runtime code.
  • elif TYPE_CHECKING: branches are type checking only, as they were with the Python import scanner.
  • A nested if TYPE_CHECKING: restores the enclosing state when it ends, rather than resetting it to "not type checking".

Graphs built without exclude_type_checking_imports are unchanged.

On the installed versions of filelock, numpy, aiohttp, propcache, docutils, anyio and packaging (with include_external_packages=True), this adds 14 runtime imports that were missing, such as filelock -> filelock._read_write, numpy._typing._array_like -> numpy._core.multiarray and aiohttp.connector -> ssl. It also removes 2 type checking only ones that were kept: packaging.version -> typing_extensions and packaging.specifiers -> typing_extensions, which are imported under elif TYPE_CHECKING:.

There's no measurable performance change. Building the graph of nine installed packages (1,920 modules, release builds, one thread, with exclude_type_checking_imports=True) takes 3,399M instructions before and 3,401M after, within the run-to-run noise of about 2M. The instructions spent in the import visitor go down by 0.4M.

  • Add tests for the change. In general, aim for full test coverage at the Python level. Rust tests are optional.
  • Add any appropriate documentation.
  • Add a summary of changes to the latest section at the top of CHANGELOG.rst. (If it's not there, add it.)
  • Add your name to AUTHORS.rst.
  • Run just full-check. (I ran the lint (Python, plus Rust with the pinned 1.97.0 toolchain), the docs build, cargo test, and the Python tests on 3.10 to 3.14. I didn't run them on 3.14t, 3.15 or 3.15t.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN

With exclude_type_checking_imports, every import inside an
`if TYPE_CHECKING:` statement was treated as type checking only, including
the imports in its `elif` and `else` branches. Those branches run when
TYPE_CHECKING is false, that is, at runtime, so their imports were wrongly
left out of the graph, and any contracts they break were kept.

Only treat the branches guarded by TYPE_CHECKING as type checking only.
This also recognises `elif TYPE_CHECKING:` again, as the import scanner
did before it was ported to Rust, and stops a nested `if TYPE_CHECKING:`
from marking the rest of the enclosing guard as runtime code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GwUvhmzgfkiZ6vo4uGyesN
@codspeed

codspeed Bot commented Oct 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 23 skipped benchmarks1


Comparing pylaterreur:fix-type-checking-else (dc2c02b) with main (676e490)

Open in CodSpeed

Footnotes

  1. 23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

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.

exclude_type_checking_imports also excludes imports in the else branch of if TYPE_CHECKING:

1 participant