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

jump to definition doesn't work sometimes for symbols imported from top level of module #733

Closed
christopherhesse opened this issue Dec 14, 2020 · 19 comments
Labels
needs investigation Could be an issue - needs investigation

Comments

@christopherhesse
Copy link

Environment data

  • Language Server version: 2020.12.2
  • OS and version: Mac OS X 10.15.7
  • Python version (& distribution if applicable, e.g. Anaconda): Anaconda Python 3.7.3

Expected behaviour

I should be able to jump to the definition of imported symbols.

Actual behaviour

I sometimes cannot do this.

Logs

[Info  - 1:45:24 PM] Pylance language server 2020.12.2 (pyright 6feebac9) starting
[Info  - 1:45:24 PM] Server root directory: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist
[Info  - 1:45:25 PM] No configuration file found.
[Info  - 1:45:25 PM] Setting pythonPath for service "pylance-import-issue": "/Users/csh/miniconda3/envs/pylance-import-issue/bin/python"
Search paths found for configured python interpreter:
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/lib-dynload
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/site-packages
  /Users/csh/Downloads/pylance-import-issue/pylance_test_package
[Error - 1:45:25 PM] stubPath /Users/csh/Downloads/pylance-import-issue/typings is not a valid directory.
[Info  - 1:45:25 PM] Assuming Python version 3.7
[Info  - 1:45:25 PM] Assuming Python platform Darwin
[Info  - 1:45:25 PM] Searching for source files
[Info  - 1:45:25 PM] Found 4 source files
[Info  - 1:45:25 PM] Background analysis(1) root directory: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist
[Info  - 1:45:25 PM] Background analysis(1) started
Background analysis message: setConfigOptions
Background analysis message: setTrackedFiles
Background analysis message: markAllFilesDirty
Background analysis message: setFileOpened
Background analysis message: getSemanticTokens
[BG(1)] parsing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (14ms)
[BG(1)] parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi [fs read 3ms] (97ms)
[BG(1)] binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi (34ms)
[BG(1)] binding: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (0ms)
[BG(1)] parsing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/constants.py [fs read 0ms] (2ms)
[BG(1)] binding: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/constants.py (0ms)
Background analysis message: analyze
[BG(1)] analyzing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py ...
[BG(1)]   checking: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py ...
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi [fs read 0ms] (11ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi (2ms)
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi [fs read 0ms] (4ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi (1ms)
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/typing.pyi [fs read 1ms] (25ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/typing.pyi (6ms)
[BG(1)]   checking: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (69ms)
[BG(1)] analyzing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (69ms)
Background analysis message: getSemanticTokens
Background analysis message: resumeAnalysis

Code Snippet / Additional information

Zip file of example code:
pylance-import-issue.zip

conda create --name pylance-import-issue python==3.7.3
conda activate pylance-import-issue
pip install -e pylance_test_package

Opening the folder as code . in the root dir:

Screen Shot 2020-12-14 at 1 46 48 PM

Jumping to CONSTANT1 or CONSTANT2 with cmd+left click works, but the symbol constants cannot be jumped to.

Opening an individual file in an empty workspace e.g. code pylance_test_package/pylance_test_package/something.py (and after setting correct environment):

Screen Shot 2020-12-14 at 1 54 03 PM

Jumping to all symbols works.

@erictraut
Copy link
Contributor

The main __init__.py file in your module doesn't re-export the constants submodule, so constants is not a symbol available within its namespace. If you want to add it to the primary module's namespace, you would need to add one of the following to __init__.py:

from . import constants
from .constants import *
__all__ = ["constants"]

If you use any of these techniques, constants will be part of the pylance_test_package module namespace, and it will work as you expect with regard to "jump to definition".

@jakebailey jakebailey added the waiting for user response Requires more information from user label Dec 15, 2020
@github-actions github-actions bot removed the triage label Dec 15, 2020
@christopherhesse
Copy link
Author

Thanks!

Here are the versions I tried, I would restart vscode each time and observe the highlighting in something.py.

# from . import constants
# __all__ = [
#     "constants",
# ]

# from . import constants
# __all__ = [
#     constants,
# ]

# from pylance_test_package import constants
# __all__ = [
#     "constants",
# ]

# from pylance_test_package import constants
# __all__ = [
#     constants,
# ]

# from pylance_test_package import *
# __all__ = [
#     "constants",
# ]

# from pylance_test_package import *
# __all__ = [
#     constants,
# ]

# from . import *
# __all__ = [
#     "constants",
# ]

from . import *

Unfortunately I still get the same issue in all cases. I did notice something though:

code pylance-import-issue/pylance_test_package <- this seems to work fine (screenshot 2) regardless of the contents of __init__.py
code pylance-import-issue <- this seems to not work (screenshot 1)

One suspicious thing here is that the folder and the python module have the same name. There's no __init__.py in the first level folder.

@jakebailey
Copy link
Member

Is your import root a subdirectory of your workspace? That's what it seems like; we assume the workspace root is where imports are rooted from unless you specify extra paths to be import roots: https://github.com/microsoft/pylance-release/blob/main/TROUBLESHOOTING.md#unresolved-import-warnings

@christopherhesse
Copy link
Author

I don't think I have an import root in the sense you mean. We have a tree of directories, some of which contain python packages, and those are installed like this package, pip install -e <package>. I originally noticed this because I would do code <repo root> to open the tree of directories. The problem seems to go away if I do code <repo root>/.. (that is, open the parent folder of the repo).

@jakebailey
Copy link
Member

Editable installs are a case we don't fully support yet, so there may be some troubles there: #78

@christopherhesse
Copy link
Author

Oh good to know. So is the preferred method that I crawl the directory tree and build up some sort of composite import root of all directories with a setup.py and write it to python.analysis.extraPaths in the workspace config?

@jakebailey
Copy link
Member

Most people only ever have to set one or two import roots; I wouldn't expect you to write tooling to make it work. Editable installs should be handled such that installing a package marks it as an import root, which is what #78 is about (we handled this in MPLS and should here, but we've been working on other things).

@jakebailey
Copy link
Member

Of course, if you do make the giant list and try it out, and it works, that'd be helpful to know as the behavior should be somewhat similar if editable installs are fixed (as that's effectively the same thing).

@christopherhesse
Copy link
Author

christopherhesse commented Dec 15, 2020

Great, thanks for the help! Should I close this issue in favor of #78? Since my packages are arranged in a tree and not a list, I think I'd need one of:

  • Configure a very large number of different import roots (possibly via a script)
  • Do a multi-root workspace where I individually add all the packages I care about
  • Open the parent directory of the tree, which oddly enough seems to fix the behavior

@christopherhesse
Copy link
Author

Actually @jakebailey I'm seeing this even if I install the package normally (pip install, no -e flag). Is it just my machine?

@jakebailey
Copy link
Member

I think I'd be interested in seeing the trace logs in that situation, specifically the bit where we print the search paths being used. If they're properly installed and in a search path, they should work, but it's a bit hard to guess based on your description the actual layout on disk.

@christopherhesse
Copy link
Author

So for this issue (not the directory tree thing, which is where I first saw this), I included a zip file of the code to reproduce the issue.

I tried again with the installed package and with trace logging enabled:

[Info  - 1:32:53 PM] Pylance language server 2020.12.2 (pyright 6feebac9) starting
[Info  - 1:32:53 PM] Server root directory: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist
[Info  - 1:32:53 PM] No configuration file found.
[Info  - 1:32:53 PM] Setting pythonPath for service "pylance-import-issue": "/Users/csh/miniconda3/envs/pylance-import-issue/bin/python"
Search paths found for configured python interpreter:
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/lib-dynload
  /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/site-packages
[Error - 1:32:53 PM] stubPath /Users/csh/Downloads/pylance-import-issue/typings is not a valid directory.
[Info  - 1:32:53 PM] Assuming Python version 3.7
[Info  - 1:32:53 PM] Assuming Python platform Darwin
[Info  - 1:32:53 PM] Searching for source files
[Info  - 1:32:53 PM] Found 4 source files
[Info  - 1:32:53 PM] Background analysis(1) root directory: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist
[Info  - 1:32:53 PM] Background analysis(1) started
Background analysis message: setConfigOptions
Background analysis message: setTrackedFiles
Background analysis message: markAllFilesDirty
Background analysis message: setFileOpened
Background analysis message: getSemanticTokens
[BG(1)] parsing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (14ms)
[BG(1)] parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi [fs read 2ms] (86ms)
[BG(1)] binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi (30ms)
[BG(1)] binding: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (0ms)
[BG(1)] parsing: /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/site-packages/pylance_test_package/constants.py [fs read 0ms] (1ms)
[BG(1)] binding: /Users/csh/miniconda3/envs/pylance-import-issue/lib/python3.7/site-packages/pylance_test_package/constants.py (1ms)
Background analysis message: getSemanticTokens
Background analysis message: analyze
[BG(1)] analyzing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py ...
[BG(1)]   checking: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py ...
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi [fs read 0ms] (10ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/2and3/_typeshed/__init__.pyi (1ms)
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi [fs read 1ms] (4ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/third_party/2and3/typing_extensions.pyi (0ms)
[BG(1)]     parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/typing.pyi [fs read 1ms] (34ms)
[BG(1)]     binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/typing.pyi (8ms)
[BG(1)]   checking: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (77ms)
[BG(1)] analyzing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (77ms)
Background analysis message: resumeAnalysis
Background analysis message: getDiagnosticsForRange
[FG] parsing: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (13ms)
[FG] parsing: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi [fs read 3ms] (88ms)
[FG] binding: /Users/csh/.vscode/extensions/ms-python.vscode-pylance-2020.12.2/dist/typeshed-fallback/stdlib/3/builtins.pyi (30ms)
[FG] binding: /Users/csh/Downloads/pylance-import-issue/pylance_test_package/pylance_test_package/something.py (0ms)
Background analysis message: getDiagnosticsForRange

@christopherhesse
Copy link
Author

@jakebailey do the trace logs reveal anything interesting here?

@jakebailey
Copy link
Member

Assuming you haven't set any extraPaths (which we aren't printing; need to work on that), not exactly. Are these packages installed into site-packages, or are they editable and are being installed via some pth file? Or, are they being installed as zipfiles (something I had forgotten is a thing in python).

(It's not likely that we'll have a massive amount of bandwidth to look at things for a while; holiday season and nearly everyone including me are about to take a break... Sorry about response time looking at these.)

@christopherhesse
Copy link
Author

That last trace was done with pip install without -e, so it looks like it's installed into site-packages. And to be clear, I am now talking only about the zip file reproduction I uploaded, not the tree of python packages I mentioned earlier. If it was a problem with installing things in editable mode, I would expect the problem to not show up when installing this single test package in non-editable mode.

Also, totally understand about bandwidth. Just wanted to figure out if it was a bug, the editable install thing, or my computer. Feel free to ignore until after the holidays.

@jakebailey
Copy link
Member

I haven't had a chance to try and run that example code yet, no. I'll switch the label so we know that it's about checking that.

@jakebailey jakebailey added needs investigation Could be an issue - needs investigation and removed waiting for user response Requires more information from user labels Dec 17, 2020
@christopherhesse
Copy link
Author

@jakebailey was this the same issue as #859 ?

@jakebailey
Copy link
Member

I'm not certain; this issue mentioned editable installs which I believe may be handled differently. My intent was to go back over all of the old import resolution / local package issues to see if the behavior changed.

Are you saying that your original issue is fixed in the most recent release? I'm very curious.

@christopherhesse
Copy link
Author

The issue also occurred with normal installs: #733 (comment)

Seems to be fixed now, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs investigation Could be an issue - needs investigation
Projects
None yet
Development

No branches or pull requests

3 participants