Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pylance not indexing all files and symbols for sqlalchemy even with package depth of 4 #4637

Closed
rbhanot4739 opened this issue Jul 21, 2023 · 11 comments
Assignees
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@rbhanot4739
Copy link

rbhanot4739 commented Jul 21, 2023

Environment data

  • Language Server version: 2023.7.31
  • OS and version: darwin arm64
  • Python version (and distribution if applicable, e.g. Anaconda):
  • python.analysis.indexing: true
  • python.analysis.typeCheckingMode: off

Code Snippet

pylance-bug

Hi,

I am using Sqlachemy orm in my project however pylance is not able to index symbols correctly and hence not providing correct suggestions in the quick fix popup. In the gif below I have AsyncSession being referred in the function signature from sqlalchemy.ext.asyncio.session package how pylance gives the incorrect quick fix suggestion of _AsyncSessionContextManager

Here is another screenshot of where the autocomplete suggestions seems to be incorrect/incomplete. As you can see the the popup does not show the select statement from sqlalchemy in the suggestion list which I am actually expecting in this case and these suggestions does not seem to context aware.

image

This is how my workspace config wrt to package indexing looks like

"python.analysis.packageIndexDepths": [
			{
				"name": "sqlalchemy",
				"depth": 4,
				"includeAllSymbols": true
			},
			{
				"name": "fastapi",
				"depth": 2,
				"includeAllSymbols": true
			}
		],

Logs

Posting logs in the attached file due to github template character limit.
pylance-logs.txt

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Jul 21, 2023
@debonte
Copy link
Contributor

debonte commented Jul 21, 2023

What version of SQLAlchemy are you using?

@rbhanot4739
Copy link
Author

What version of SQLAlchemy are you using?

Version 1.4.28

@debonte
Copy link
Contributor

debonte commented Jul 22, 2023

Reassigning to @heejaechang.

I believe this only repros in 2023.7.31 (or later) where the sqlalchemy stubs have been removed from typeshed.

I was able to repro this by doing the following:

  1. Create new directory with venv
  2. Activate venv
  3. pip install sqlalchemy==1.4.28
  4. Open that folder in VS Code
  5. Create .vscode\settings.json configuring python.analysis.packageIndexDepths as shown above.
  6. Create test.py
  7. Type in x: AsyncSession
  8. Click on AsyncSession
  9. Click light bulb

I thought what was happening was:

The indexer sees the AsyncSession import in sqlalchemy\ext\asyncio\__init__.py and prefers that location over the original decl in sqlalchemy\ext\asyncio\session.py because the __init__.py has a shorter module path. However, the __init__.py uses from .session import AsyncSession instead of from .session import AsyncSession as AsyncSession so we don't consider it to be part of the module's public interface and therefore don't show a code action for it.

Based on that I wasn't sure if this was simply a library bug (__init__.py should use from .session import AsyncSession as AsyncSession) or an indexer bug (indexer should notice that the AsyncSession import in __init__.py doesn't use a supported pattern and prefer the decl in session.py instead).

But then I realized that if I rename my sqlalchemy.json local_indices cache file and allow the indexer to regenerate it, the problem goes away.

So maybe there's an inconsistent bug where the cache file is misgenerated? Can you take a look? This is the first time I've dug into the indexer and I may be misinterpreting what's happening.

I'm attaching my problematic sqlalchemy.json file in case that's interesting.

sqlalchemy.json.txt

@debonte debonte assigned heejaechang and unassigned debonte Jul 22, 2023
@rbhanot4739
Copy link
Author

@debonte thanks for looking into it. However I have seen this issue with multiple modules in sqlalhemy as well as some other packges as well where the auto import suggestions are incorrect.

@heejaechang
Copy link
Contributor

@debonte I think, your indices are all messed up due to some reasons, for such cases, we have clear cache command, please run that command
image

so it can re-create the cache from scratch again. for users who doesn't want to get into such cache corruption issue, they can disable "python.analysis.persistAllIndices": false so indices are re-created every time not worrying about any cache invalidation or corruption issue.

@heejaechang
Copy link
Contributor

I think there is a bug around persisted indices, you can turn it off for now python.analysis.persistAllIndices and I think it will work as expected. thank you for reporting the issue.

@heejaechang heejaechang added bug Something isn't working and removed needs repro Issue has not been reproduced yet labels Jul 24, 2023
@rbhanot4739
Copy link
Author

rbhanot4739 commented Jul 25, 2023

image

@heejaechang I basically ran both the commands not sure which one did the trick but it worked for me, however after setting python.analysis.persistAllIndices: false and I noticed drastic performance degrade in and the editor started to stutter while typing.

Also do you have any eta on when "python.analysis.persistAllIndices" will be fixed, and is it different from "python.analysis.indexing" which I believe it was added in last couple of releases ?

@heejaechang heejaechang added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Jul 25, 2023
@heejaechang
Copy link
Contributor

@rbhanot4739 fix will be in the tomorrow's prerelease bit. python.analysis.indexing is whether to turn on or off indexing and persistAllIndices is whether to save and reuse data from previous indexing or not.

the bug was at the reusing. so as long as you turn off persistAllIndices for now, it should be fine.

@rchiodo
Copy link
Contributor

rchiodo commented Jul 26, 2023

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

@rchiodo rchiodo closed this as completed Jul 26, 2023
@nielsuit227
Copy link

nielsuit227 commented Mar 27, 2024

I'm experiencing this issue with Pylance v2024.3.2 and sqlalchemy 2.0.29. I tried to resolve with the indexing steps mentioned above, but to no avail. I have the issue both on Windows and Mac, Python 3.9, 3.10.

@debonte
Copy link
Contributor

debonte commented Mar 27, 2024

@nielsuit227, this issue has been closed for a long time, so it's likely you are seeing something different. Please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

5 participants