[ty] Support import <namespace> and from <namespace> import module
#18137
Conversation
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
MichaReiser
commented
May 16, 2025
Contributor
|
This comment was marked as resolved.
This comment was marked as resolved.
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
2 times, most recently
from
218e122 to
5cbbc47
Compare
Contributor
|
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
4 times, most recently
from
5128eb5 to
b228b70
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
MichaReiser
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Member
Author
|
Something we could try is to change |
MichaReiser
changed the title
import <namespace> and from <namespace> import module"import <namespace> and from <namespace> import module
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
from
b228b70 to
9dd1436
Compare
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
from
9dd1436 to
b02feb5
Compare
MichaReiser
commented
May 19, 2025
sharkdp
reviewed
May 19, 2025
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
from
1d9f6c3 to
4552889
Compare
Member
|
The import foo
from foo.bar import bazthen an implicit |
Member
This seems like a true positive...? % uv run --no-project --with=mitmproxy python
Built pyperclip==1.9.0
Installed 44 packages in 45ms
Python 3.13.1 (main, Jan 3 2025, 12:04:03) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from mitmproxy.test import tutils
>>> tutils.test_data
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
tutils.test_data
AttributeError: module 'mitmproxy.test.tutils' has no attribute 'test_data'I don't really understand what's (meant to be) going on in mitmproxy's |
AlexWaygood
reviewed
May 19, 2025
AlexWaygood
reviewed
May 19, 2025
AlexWaygood
reviewed
May 19, 2025
AlexWaygood
reviewed
May 19, 2025
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
The primer hits look great!! I left some comments in resolver.rs -- not sure about some of the details there
MichaReiser
commented
May 20, 2025
Member
Author
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
Hmm, I just noticed that my comments are still pending
Member
Author
|
@AlexWaygood Is this good to go once I improved the comment or is there something else? |
AlexWaygood
approved these changes
May 20, 2025
MichaReiser
force-pushed
the
micha/namespace-package-import
branch
from
4552889 to
e4b3531
Compare
|
I have what seems like a related issue -- tried with latest ty but seems like it's still happening. Wondering, has this fix made it into Happy to file a separate issue in ty bugtracker if this is somehow unrelated! What happensConsider the following hierarchy: This is the only file with code: This results in: Expected behaviour
|
|
@karlicoss I don't believe a new version with this fix has been released yet. |
Contributor
|
@karlicoss Should be available in ty 0.0.1-alpha.7. |
|
Thank you, that was fast! Can confirm it works |
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
The module resolver incorrectly returned
Noneif a module name resolves to a namespace package (it did correctly handle the case where a module name resolves to a file inside a namespace package).Supporting this requires a smaller refactor because
Moduleassumed thatfileandsearch_pathare always known. However, neither are present for a namespace package (it doesn't resolve to a file AND the package can exist on multiple search paths).Fixes astral-sh/ty#375
Fixes astral-sh/ty#363
Test plan
Added regression tests. I verified that the imports in the linked issues resolve successfully.
Ecosystem review
optuna: Correct,auto_generatedis a namespace packagerotki: Correct, the project has apackagingfolder. It doesn't contain any python code but it isn't wrong for ty to pick it up as a namespace package.psycopgpsycopg/psycopg(it's a src layout but thesrcfolder is calledpsycopg). We weren't able to resolve any imports before. We now resolvepsycopgas a namespace package which is correct from a module resolution point. The solution here is to change how we detectsrc.rootor that they change their configuration to setsrc.rootrclip: Is a namespace package and we are now able to resolve the importsdd-trace-py:benchmarksis a namespace package and we're now able to resolve imports. They also use other namespace packages that now resolve correctlyscipy: They use submodules for some of the modules in_libbut they point to the project root and not the package folder? We now recognize them as namespace packages.Unsure:
schema_salad:The relevant lines are:
ruamelis a namespace package,ruamel.yamlis a regular package.Before: We inferred
UnknownforruamelandyamlNow: We infer
ruamel.yamlbut it seems we don't resolve thecomments.pymodule?I think this is unrelated to my changes.
cwltool: Same as
schema_salad