Repository navigation
fix(python): a script's import of a module beside it in a package folder links again - #3155
Merged
Merged
Conversation
…der links again 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 beside it links again when the folder also holds an
__init__.py. On rocksdb, main still lost 45 of the 49 true edges the regression hunt listed against 1.6.2, and this brings them back. #3141 had already restored the other 4.This is a regression from #1921, which hasn't shipped, in the same function as #3141.
Root cause
Confirmed in the code and on the hunt's fixture at 0f93575.
resolvePythonModuletakes the importer's own folder as a root only when!index.packageDirs.has(dir). That's right for a real package: a barefrom utils import xmust not take a sibling over the true top-level module.It also loses this layout. rocksdb's
tools/block_cache_analyzer/block_cache_pysim_test.pydoesfrom block_cache_pysim import (...)besideblock_cache_pysim.py, in a folder that holds an__init__.py. A script run from there has that folder first onsys.pathall the same.Fix
The fallback from #3141 now starts at the importer's own folder. It's renamed
pythonModuleOfScript, since it no longer only looks above.django/utils/text.py, whosefrom html import escapewould have takendjango/utils/html.py.sys.stdlib_module_namesstays the stdlib's, above a script as well as beside it. That narrows fix(python): a script's import of a module in a folder above it links again #3141 slightly: a projecthtml.pyabove a script no longer answersfrom html import escape. No repo below has that case. CPython'stestpackage isn't on the list, so redis'sfrom test import TestCaseis unaffected. The list is the public names in CPython'sPython/stdlib_module_names.h, so a module removed before that version, likedistutils, is judged like any other name.pkg/client.pydoingfrom client import Client), so a file is never its own answer.mypy/types.py. Stdlib-named imports don't count. This also closes the gap fix(python): a script's import of a module in a folder above it links again #3141 listed, a dotted module above a script, and a module that is a package, which had the same gap.The
[Unreleased]bullet from #1921 gains a clause. No new bullet, because the regression never shipped.Validation
Tests: 20 new in
python-module-resolution.test.ts, 10 of them on sync, which compare against a fresh index both ways. Against main's source 10 fail: the fixture's shape, the dotted form, a stdlib name above a script, and the sync cases, including #3141's two gaps. The other 10 pin what must not link or re-open, and pass on both. Here all 79 pass. Taking out each of 19 guards in turn fails a test, including both new checks, each scope branch and the re-open itself.The hunt's fixtures:
py6goes from 6 edges to 11: theruncall, theHashTableinstantiation, the 3 imports, andHashTable::sizeas a typed call rather than a name guess.py1topy5are unchanged.A/B against main, kernel on, at the base 0f93575:
server/polar, djangodjango/, scikit-learn, matplotlib, redis, and 4 small appsBoth controls have byte-identical full dumps. redis keeps the 77,781 edges #3141 gave it.
The edit-scope rewrite came after that A/B and only touches sync. Rerun on this commit against its base d87ec36, requests, flask, mypy, redis, django
django/, litestar, express and gin are all unchanged, with byte-identical control dumps, andpy6still has its 11 edges.Every change, all in rocksdb and all from
block_cache_pysim_test.pytoblock_cache_pysim.py:importsof its classes, functions and variablesinstantiatescallsofcreate_cacheandrunimportsof the fileChecked against the hunt's list of 49: 45 back here, and the other 4 (
decode_instruction, one level up, a plain folder) already on main from #3141.A real-tree sync: on rocksdb's own folder, renaming
runand syncing matches a fresh index, and restoring it matches the first index. In-function andtry:imports converge the same way.Nothing wrong appears. The trace logged every candidate the fallback could see, on the repos above and on whole rocksdb, django, polar's server, ansible and cpython. pydantic wasn't traced. Above a script, the only hits are the two on main: redis's
test.pyand rocksdb'sinjector_critical_instruction.py. In a file's own folder, these are all the hits the fix keeps:block_cache_pysim_test.pyblock_cache_pysimtests/proxy_model_inheritance/tests.pyapp1.models,app2.modelssys.pathwithextend_sys_pathLib/idlelib/editor.pyutilLib/zoneinfo/__init__.py_zoneinfoexceptbranch binds the same class through._zoneinfo, so nothing false is addedEvery other candidate is a stdlib name, a third-party package's name, or a plain import, and none binds.
django/utils/text.pyis the one stdlib name the proof alone would have let through.Cost: nothing measurable. Median of 3 interleaved index runs: redis 12.8 s on main, 13.8 s here, with fastest runs of 12.39 s and 12.34 s; mypy 10.0 s and 10.2 s, fastest 9.87 s on both. The edit scope, timed directly with the resolver's caches cleared each of 7 rounds: sqlalchemy 2.9 ms on main and 2.7 ms here, cpython 8.4 ms and 8.3 ms, django 16.8 ms and 17.0 ms. A first version read every import node there, which cost 70 to 110 ms per sync, so it looks them up by name instead.
Suite: 854 files (848 passed, 6 skipped), 10,535 tests passed, 2 expected failures, 0 failed, 87 skipped, on this exact commit, kernel on and rebuilt for this base, 4 workers, Vitest 4.
Not in this PR
import helperandfrom helper import *beside a file take no name, so they prove nothing and stay unresolved, as in fix(python): a script's import of a module in a folder above it links again #3141. Every plain-import candidate in the trace was a stdlib name, a third-party package's name,__main__, or a receiver name rather than an import, so this costs no true edge on these repos.from queue import MyQueuebesidepkg/queue.py. Telling those apart would take the stdlib's own export lists. It's a missed edge, never a wrong one, and no repo above has a case.pkg/redis.pywith its ownRedisnext to a file that means the real one. Nothing in the tree separates the two. No repo above has a case.🤖 Generated with Claude Code