[ty] Exclude test-related symbols from non-first-party packages in auto-import completions#23252
Conversation
Ideally we'd just have one helper instead of two, but adding site-packages support creates a lot of fallout with tests using the helper because of the change in file paths.
BurntSushi
requested review from
AlexWaygood,
Gankra,
MichaReiser,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
BurntSushi
removed request for
Gankra,
MichaReiser,
carljm,
dcreager,
ibraheemdev and
sharkdp
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
Nice! (N.B. I haven't pulled this one locally and played around with it, but it makes sense to me)
| name = "test" | ||
| version = "0.1.0" | ||
| requires-python = ">=3.13" | ||
| dependencies = ["pandas>=2.3.3"] |
There was a problem hiding this comment.
I'd sort-of be tempted to construct a mock package for this test, rather than rely on pandas, which could always change where its tests are located in the future? But maybe the uv.lock file protects against that kind of breakage...
There was a problem hiding this comment.
I think the lock file protects us here. There's always a risk that pandas gets updated and the test is no longer testing what we think it's testing though. On the other hand, I do think it's important to use real projects sometimes instead of mocking out what you think you need.
| // Test functions (starting with `test_`) in third-party | ||
| // packages are almost never useful to import. | ||
| if is_non_first_party && symbol.name.starts_with("test_") { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
if a library uses unittest rather than pytest, it probably won't have any module-level test_foo_bar() functions, but instead it may have lots of module-level FooBarTests classes. Is it worth also filtering out CamelCase names that end with Tests?
Unlike test_foo_bar() functions with pytest, classes with such names aren't automatically detected as test classes by unittest -- you need to have unittest.TestCase somewhere in the class's MRO. But it's a very common naming convention among unittest users.
(This could also be a separate followup change; doesn't need to block the PR.)
There was a problem hiding this comment.
I haven't seen those come up often in completion results. So yeah I think this might be better suited for a follow-up if it becomes an annoyance.
AlexWaygood
added
server
…to-import completions This reduces noise when typing, as test code from dependencies is almost never useful to import. Note that first-party code is never filtered, so that users can still import from their own test modules. Top-level "testing" modules like `pandas.testing` are preserved since they provide utilities meant for external use. Closes astral-sh/ty#2466
BurntSushi
force-pushed
the
ag/claude-auto-import-no-tests
branch
from
8e101b8 to
d08268a
Compare
2 participants
This reduces noise when typing, as test code from dependencies is
almost never useful to import.
Note that first-party code is never filtered, so that users can still
import from their own test modules. Top-level "testing" modules like
pandas.testingare preserved since they provide utilities meant forexternal use.
Closes astral-sh/ty#2466