Autoimport from module, not directly from file

[last-partizan]

I use pyright in django project with models structured like this:

appname:
  - models:
    -  __init__.py
    - book.py
    - author.py

__init__.py contains all imported models and __all__ = ... variable.

When using autoimport feature, it suggests importing from "appname.models.book", but i want it to use "appname.models".

When importing from django, for example: Count, it suggests two variants: from django.db.models and from django.db.models.aggregates, so i can select preferable variant. But not for my code.

Any suggestions? Is there something wrong with my code? Maybe that's a bug and it could be fixed?

[erictraut]

Pyright's auto-import feature suggests symbols that it has already encountered and cached in memory. That means you need to have already opened a file that imports from that location or run the type checker on such a file (e.g. by setting diagnosticMode to "workspace"). If it sees the same symbol imported from multiple locations, it will always suggest the shorter module path. In your case, it has probably only seen the longer path, so that's what it suggests.

I recommend switching to Pylance if you're using VS Code. Pylance incorporates a symbol indexing mechanism so it quickly discovers all importable symbols. Its auto-completions will therefore be more accurate and deterministic than pyright's.

[last-partizan]

@erictraut no matter what i use, nvim + pyright-langserver, or vscode + pylance, there is no option to import from shortest path.

Here you can see app/views is opened and uses short import, but pylance suggests importing from long path. The same behaviour is observed with nvim + pyright.

In both cases i use diagnosticMode="workspace".

You can try it yourself, here's minimal example, open app/tests.py and try autoimporting Book.

https://github.com/last-partizan/pyright-examples/tree/235efaec380e4ee17b3011eba63354f805cc3ddb

[erictraut]

Thanks for the example. I'm able to repro.

I'm going to reopen this bug and transfer it to the pylance project. The pylance team is primarily responsible for all of the language service features in pyright.

[erictraut]

@debonte, could you please transfer this issue to the pylance-release project? It appears to be a bug in the completion provider.

[heejaechang]

looking at it.

[heejaechang]

I just let import alias from user file show up in add/auto import when indexing is off since we don't do import alias deduplication when indexing is off (indexing is always off for pyright)

when indexing is on, things will still work as it used to. we will investigate more to see whether the cost is worth the benefit.

[debonte]

This issue has been fixed in prerelease version 2023.3.41, which we've just released. You can find the changelog here: CHANGELOG.md

[last-partizan]

Are you sure it's fixed?

I switched Pylance to pre-release version, and it still does not allow me to import from shortest path.

Does this fix apply to pylance only, or it should work in pyright 1.1.301 also? (I mostly use pyright as langserver with neovim, pylance only for testing this bug).

[heejaechang]

@last-partizan as I wrote here, that behavior is only enabled when python.analysis.indexing: false. in pyright, it is always off.

[heejaechang]

please, remember with indexing off, all import alias in all user files will show up. in another words, if you have

# file1
def Book(): pass

# file2
from file1 import *

# file3
from file2 import *

# file4
from file3 import *

you will get auto import of Book from file1, file2, file3 and file4

[last-partizan]

Indexing is enabled, i'm using latest pre-release version of Pylance (v2023.3.41).

But still, autoimport and quickfix are suggesting using long import, instead of shortest one. You can try it yourself on my example mentioned above.

For django, for example, i can see the difference. In neovim, pyright is suggesting to import "HttpResponse" from django.http and django.http.request. But in vscode Pylance suggesting only django.http.

So, it doesn't work for user code, only for installed modules?

[heejaechang]

@last-partizan as I said in the comment (https://github.com/microsoft/pylance-release/issues/4065#issuecomment-1482010087), change is only for when indexing is off.

there is no change when indexing is on. our existing behavior when indexing is on is we never suggest import alias from user files.

[last-partizan]

Oh, my bad. Somehow i was reading it as "only when on".

Yes, now it works as expected, thank you.

[heejaechang]

@last-partizan we got push back from people who didn't like the new behavior. so for now, we just reverted the change. we probably need more time to think about how to support it.

[BuckAMayzing]

FWIW, I just spent a while trying to figure out how to get this to work as I expected (the new behavior), so I'm also on the pre-release and running with indexing off.

I'm also curious: why would this behavior only change for the case where indexing is turned off?

Suggestion: perhaps if there is push back on the behavior, maybe a setting along the lines of python.analysis.addImport.includePackagePaths could be added? Or alternatively, if there are multiple path sources, maybe an array of enums for those sources could allow customization both for what is included and what order they would show up in?

[heejaechang]

@BuckAMayzing sure, we can do that. but we will gather more user feedback before we try to address it again.

By the way, the previous fix we reverted was the cheapest fix we could do, any other ones, it could be expansive relatively since they all require some kind of deduplication of import symbols.

[last-partizan]

@heejaechang by the way, what sort of feedback you were receiving?

I cannot imagine why people would prefer to import symbols from files where they defined, instead of top-level module.

Or it was unrelated to this exact change and just broken something else?

This part of code is open, and in pyright? Can you point me at where exactly?

[erictraut]

I think your assumption may be incorrect. I think that most of the time, people want to import symbols from the module where they are defined within the local project. Re-exporting symbols from other submodules is not typical within "app" code. It's more common in library code, and pyright already handles this case fine. So, I think what you're doing here is a bit atypical. I'm curious why you're importing and then re-exporting symbols within your own code base. Why not import them directly from the module where they are defined?

The way @heejaechang implemented your feature request initially caused the auto-import feature to suggest imports for symbols that were imported from libraries and then used in local files. For example, if you import from typing import TypeVar in a local file foo.py and then go to a second local file bar.py, you would not expect auto-import to suggest from foo import TypeVar. You'd expect this symbol to be imported from its original library (from typing import TypeVar). This change made auto-import very dangerous to use for any symbol imported from a library because it started to suggest that such symbols be imported from local files instead. It destroyed the utility of auto-import. I know that's not what you intended with your feature request.

[heejaechang]

@last-partizan so, we already show import alias from third party as you showed in your example django, what we don't do is not doing so if it is from user files (the local project).

In local project, I believe people usually directly reference the symbol from the module it is defined in since otherwise (if it uses re-exported symbols), at runtime, all those files that re-exports symbols are needed to be brought into memory and processed. library having those are understandable to have better public API signatures, but for internal symbols used in implementation, I believe people usually reference them directly.

anyway, to improve the situation, we need to resolve all imported aliases (meaning bring in all related files into memory and parsed/bound transitively) used in user files, and process them (dedup, filter out and etc). and that is perf hit.

so, the feedback I am trying to gather is whether it is common enough for us to spend time to implement that deduplication and it is worth to take that perf hit on either start up (indexing), completion (auto import) or code action (add import)

the naive implementation I had before was not doing the "deduplication" making it cheap comparably (only hit we get is having more items in the completion)

[last-partizan]

@erictraut my use case - is with django models.

For simple cases it's fine to put all models in single models.py, but for large apps and large models it's better to have each model in it's separate file.

Reexporing them at module level (__init__.py), is enough for Django to detect them, and importing from app.models rather than from app.models.file is lot cleaner.

@heejaechang maybe you can use __all__ variable for this purpose? That's exactly what is for - for marking publicly available members of the module/file, and django (and probably other libraries) doing this when reexporing.

[erictraut]

importing from app.models rather than from app.models.file is lot cleaner

I don't think I understand what you mean by "cleaner". I admit that the import statement is shorter, but length isn't really an issue when auto-import does all of the work for you. I presume it's just a personal preference — i.e. what you're used to seeing in your code.

[last-partizan]

It may be presonal preference, but it has practical benefits:

• when refactoring i can freely change file names, and change only single import instead of many through the app code
• it's easier to read

[debonte]

when refactoring i can freely change file names, and change only single import instead of many through the app code

For what it's worth, when you rename files Pylance should automatically (or at least offer to) fix your imports.

[heejaechang]

@last-partizan let's see how many people wants this. If enough people want it, then we can revisit this. and we can think through more details at the time such as whether we need new option or not, whether it should be on by default or not, how much time it will add to indexing/completion/code action time, how many more files we need to parse/bind in avg and etc.

[shayded-exe]

In my case, I'm developing multiple inter-dependent libraries in a monorepo. I want Pylance to only auto import from the top level of each library, but it doesn't work.

[heejaechang]

this is related to https://github.com/microsoft/pylance-release/issues/5434