Open binste opened 1 year ago
@binste, I'm unable to repro this issue using Pylance 2023.4.11. I confirmed that without your changes, selection
is shown in the completion list. However, after applying your changes to __init__.py
and adding a py.typed
file, I no longer see it.
Here's what I did:
python -m venv .venv
.venv\Scripts\activate
pip install altair
which gave me v4.2.2__init__.py
with the version in your branchpy.typed
file.py
fileimport altair as alt
alt.sel
and I see this:
Are you still seeing this? If so, can you help us repro it?
Thank you @debonte for looking into it. I followed every step of your instructions and I still see selection
showing up. I'm now on Pylance 2023.4.30 and VS Code 1.77.3.
Here's the location of the py.typed file I created in case that would be wrong:
As you can see in the video above, selection
is greyed out and so Pylance seems to be aware that it is not accessed in __all__
.
I also restarted VS Code and executed the Python: Restart Language Server
command but to no effect. I'm no longer running this in a Docker container but instead directly on a Macbook. I will test this as well on a Windows machine in the coming days.
@binste, I'm still unable to repro this. I tried both Windows and Linux.
Are you sure that you have the right virtual environment selected in VS Code?
If so, can you please set "python.analysis.logLevel": "Trace"
, then repro the issue, and provide the resulting "Python Language Server" log from the Output pane?
Sorry that it took me a while to get back to this. Some context for the log: I opened the project again, updated all extensions, then reopened the window, verified that the interpreter is set to the correct one in the .venv
folder, set the loglevel to trace in the workspace settings file, opened test.py
, and imported altair and typed alt.selec
and selection
showed up.
Can you post the contents of the following file, assuming you haven't changed it since you collected the log above?
(40209) [FG] binding: /Users/stefanbinder/Library/CloudStorage/Dropbox/Programming/altair_playground/vscode_completion/.venv/lib/python3.10/site-packages/altair/__init__.py
Sure, the file is still the same as when I collected the log:
@erictraut, this issue is related to interim files. Can you give me the executive summary of what their purpose is?
Interim files always have _isThirdPartyPyTypedPresent
set to false
, which causes the binder to not call setPrivatePyTypedImport
for symbols that aren't in __all__
. When the bug repros we're getting the module symbol table from the interim SourceFile
for the altair
__init__.py
.
The concept of an "interim file" (initially misspelled as "intrim file") was introduced via a sync with pylance on Jan 4, 2023. I don't know why it was added. There are no comments in the program.ts module that explain the concept. Since the change came in as part of a sync from pylance, I can't tell who initially wrote this code. The intent behind the change is obscured because it's combined with a bunch of other changes, and there are no detailed commit comments. I'm guessing that Heejae made this change based on the coding style and the wording in the comments.
I believe interim files were added for the pytest support. They were used to allow declarations for 3rd party libraries to be referenced when finding fixtures.
Looks like it evolved out of the "shadowed file" concept in https://github.com/microsoft/pyrx/pull/3016/files#diff-c31e28784be62cd19c158d342a0ad07bd73858853a14f1f78b2931045cf1e8f5.
Shadowed files also always had _isThirdPartyPyTypedPresent
set to false
. Not sure if that was safe for some reason or already a source of bugs.
@erictraut, this issue is related to a Teams discussion that you recently had with HeeJae.
As he mentions there, we have "features that open library files through some implicit relation rather than explicit import statements." The "interim file" mechanism is used to add these files to the program. But currently it doesn't know what values the new SourceFile
should have for isThirdPartyPyTypedPresent
and isThirdPartyImport
, so they are always false
.
In this particular issue, the feature that causes _createInterimFileInfo
to be called is module name completions. The interim SourceFile
is created as you type the "altair" portion of import altair as alt
. This repros in Pyright also.
HeeJae's proposal in the Teams chat above was to modify the return value of ImportResolver._getModuleNameForImport
to include these flag values. Possibly by reusing the pyTypedInfo
logic in _resolveAbsoluteImport
to calculate isThirdPartyPyTypedPresent
.
Does this seem like a reasonable approach?
I'd need to review a bunch of code and page this all back into my head to answer your question. I probably won't have time to do that in the next week, so don't wait for me. This sounds like a reasonable approach — worth exploring.
Repro steps:
python -m venv .venv
.venv\Scripts\activate
(or equivalent)pip install altair==4.2.2
.venv\lib\site-packages\altair\__init__.py
with the version here.venv\lib\site-packages\altair\py.typed
file.py
fileimport altair as alt
-- You must type it rather than pasting! Well, at least the altair
module name. I've found that I need to type it at a fairly good clip (normal typing pace I guess). If I type it too slowly the issue doesn't repro. I believe this is because _removeUnneededFiles
kicks in and deletes the interim file.alt.selection
Expected: The only suggestions starting with lowercase selection
should be selection_interval
and selection_point
, since they are the only ones in altair's __all__
collection.
Actual:
selection
, selection_multi
, and selection_single
are also included:
While trying to make Altair a typed library, we were trying to hide deprecated functions from the code completions on the top-level package. The approach was discussed here https://github.com/microsoft/pylance-release/discussions/3709#discussioncomment-5093881. However, Pylance still shows all symbols in the code completion even though some are no longer public exports, see https://github.com/altair-viz/altair/pull/2927 for the proposed code changes, expected and actual behaviour as well as screenshots.
In case these two threads do not provide enough context or are too complex to go through let me know and I can try to make a more minimal example.
Environment data