Repository navigation
fix(python): a script's import of a module in a folder above it links again - #3141
Merged
Merged
Conversation
… again PR #1921 made Python imports resolve from the package roots a project has, and stopped binding by any file whose path ends in the module's name. A folder that is neither a root nor the importer's own is skipped on purpose, so this common script layout lost its links: redis's modules/vector-sets/tests/*.py do from test import TestCase, and modules/vector-sets/test.py is the runner whose folder is first on sys.path. No root says so. On redis, main lost 137 true edges (69 imports, 68 calls) that 1.6.2 had. As a last step, resolvePythonModule now walks the importer's ancestors for the module, nearest first and passing over packages. A name like test or utils may just as well be the stdlib's, so what proves the module is what the importer takes from it: every name must be one the file defines at module level. A plain import or a star import takes no name, so it proves nothing and stays unresolved, as on main. A module a root provides still answers first. Reading the importer's own from-imports keeps this inside the resolver, so every caller gets one answer: context.resolveModuleFile finds the module too, and the outside-the-project check from PR #3109 does not count it as outside. A sync follows it. Such a module binds only while it defines those names, so an edit can link or unlink importers though no file appeared or vanished. An edited module in a folder that is neither a package nor a root, with Python folders below it, now re-opens the edges into it and the failures naming it. Any other edit re-opens nothing. Validation: - 9 new tests in python-module-resolution.test.ts, 2 of them on sync. Without the fallback the 4 that expect a link fail, and the 5 that expect none pass, as they should. Here all 9 pass. Taking out each guard (every name, plain imports, packages passed over, the nearest module with the names, the re-open on edit) fails exactly its own test. - A/B against main on 14 Python repos plus express and gin, 741,560 edges: 173 added, 0 removed, 0 moved, 44 extends upgraded from exact-match to import, all in redis. All 137 edges 1.6.2 had and main lost are back. The other 36 added are the file-level imports of test.py, as a from-import from the same folder gets. The controls' full dumps are byte-identical. - A trace of the fallback found every candidate it could see on those repos and on whole django, polar's server, ansible and cpython: redis's test.py from its 36 test files, all proven, and nothing else, so no wrong edge and no rejected candidate. No plain or star import had a candidate, so leaving those unresolved costs nothing on these repos. - Index cost on redis, median of 3 interleaved: 12.0 s on main, 10.7 s here, which is noise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 31, 2026
…der links again (#3155) PR #1921 made Python imports resolve from the package roots a project has. A file's own folder counts as a root only when it holds no __init__.py, because in a real package a bare `from utils import x` must not take a sibling over the true top-level module. That also lost this layout: rocksdb's tools/block_cache_analyzer/block_cache_pysim_test.py does `from block_cache_pysim import (...)` beside block_cache_pysim.py, in a folder that holds an __init__.py. A script run from there has that folder first on sys.path all the same. The fallback from PR #3141 now starts at the importer's own folder, still after every root has answered, so a top-level module a root provides keeps winning. A package above the importer stays passed over. Proving the names is not enough beside a file. A trace of every candidate found that most bare names with a module beside them are the stdlib's, and the proof passes on django/utils/text.py, whose `from html import escape` would have taken django/utils/html.py. So two checks now come with it. A name in sys.stdlib_module_names stays the stdlib's, above a script as well as beside it; CPython's `test` package isn't on that list, so redis's `from test import TestCase` is unaffected. And a module that imports its own name wraps the real one, which is what that name then means. That covers a wrapper's neighbours and the wrapper itself, so a file is never its own answer. A sync follows it. An edit re-opens a module's edges only if an import node spells that module from a folder that can reach it: a file in a package folder, for the path from that folder, or a file anywhere below a plain folder that isn't a root. Those spellings are looked up by name, one per folder above the module, so an edit nobody imports that way re-opens nothing, even for a hub like mypy/types.py, and costs nothing measurable. It also closes the gap PR #3141 left open, a dotted module above a script, and a module that is a package. The [Unreleased] bullet from PR #1921 gains a clause. No new bullet, because the regression never shipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
A Python script's import of a module in a folder above it links again, when that module defines every name the script takes from it. On redis, main lost 137 true edges that 1.6.2 had, and this brings all of them back.
This is a regression from #1921, which hasn't shipped.
Root cause
Confirmed in the code. #1921 made imports resolve from the package roots a project has, instead of any file whose path ends in the module's name.
resolvePythonModuletries the roots containing the importer, and an ancestor that is neither a project root, a local root nor the importer's own folder isn't one of them, on purpose.That loses this layout: redis's
modules/vector-sets/tests/*.pydofrom test import TestCase, generate_random_vector, andmodules/vector-sets/test.pyis the runner whose folder is first onsys.path. Nothing in the tree says that folder is a root.Fix
As a last step,
resolvePythonModulewalks the importer's ancestors for the module, nearest first, passing over packages.testorutilsmay just as well be the stdlib's, so every name the importer takes must be one the file defines at module level.from time import sleepdoesn't take a projecttime.pythat has nosleep.import helperandfrom helper import *. They take no name, so they prove nothing and stay unresolved, as on main. No repo below has a candidate of either kind, so this costs nothing measured.context.resolveModuleFilefinds the module too, and the outside-the-project check from fix(python): a base imported from outside the project is not a project class of that name #3109 and fix(python): a class-name call doesn't follow a base imported from outside the project #3115 doesn't count it as outside. There's a test for that: a class inheriting fromTestCasereaches its method.The
[Unreleased]bullet from #1921 gains one sentence. No new bullet, because the regression never shipped.Validation
Tests: 9 new in
python-module-resolution.test.ts, 2 of them on sync, both ways against a fresh index. Without the fallback the 4 that expect a link fail, and the 5 that expect none pass. Here all 9 pass. Taking out each guard (every name, plain imports, packages passed over, the nearest module with the names, the re-open on edit) fails exactly its own test.A/B against main, kernel on:
server/polar, djangodjango/, scikit-learn, matplotlib, and 4 small appsBoth controls have byte-identical full dumps.
Every class of change, all in redis:
importsofTestCase,generate_random_vectorandfill_redis_with_vectorscallsofgenerate_random_vectorandfill_redis_with_vectorsimportsof the filetest.pyextendsTestCase,exact-match→importChecked against the regression hunt's list: 137 of 137 are back.
Nothing wrong appears. A trace of the fallback logged every candidate it could see, on all the repos above and on whole django, polar's server, ansible and cpython, whose trees hold 66 files named
test.py,utils.py,common.pyorhelpers.py. It fired only for redis'stest.py, from its 36 test files, with every name proven and no candidate rejected.Cost: redis, median of 3 interleaved runs: 12.0 s on main, 10.7 s here. That gap is noise, so nothing measurable.
Suite: 849 files (843 passed, 6 skipped), 10,389 tests passed, 2 expected failures, 0 failed, 87 skipped, on this exact commit, kernel on, 4 workers, Vitest 4.
Not in this PR
__init__.pyisn't proven: only the file's own module-level defs count. Following re-exports here would recurse through the resolver. No repo above has the case.from helpers.db import connect, is found by the same walk, but an edit to it doesn't re-open its importers; only a module directly in such a folder does. No repo above has this case either.🤖 Generated with Claude Code