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

vscode/pylance doesn't recognise imports from mesopy editable installs #555

Closed
dstansby opened this issue Jan 2, 2024 · 8 comments
Closed
Labels
wontfix This will not be worked on

Comments

@dstansby
Copy link

dstansby commented Jan 2, 2024

I'm using vscode with a project using the mesonpy build backend that I've pip installed, specifically with:

python -m pip install --verbose --no-build-isolation --editable ".[dev]"

The pylance plugin in vscode doesn't seem to pick up the editable install though, and therefore can't do any static analysis (like resolving classes/functions/methods or doing type checking) with the code base.

This is likely the same issue as microsoft/pylance-release#3473, which links to this part of the pylance docs:

PEP 660 enables build backends (ex. setuptools) to use import hooks to direct the import machinery to the package's source files rather than using a .pth file. Import hooks can provide an editable installation that is a more accurate representation of your real installation. However, because resolving module locations using an import hook requires executing Python code, they are not usable by Pylance and other static analysis tools. Therefore, if your editable install is configured to use import hooks, Pylance will be unable to find the corresponding source files.

If you want to use static analysis tools with an editable install, you should configure the editable install to use .pth files instead of import hooks. See your build backend's documentation for details on how to do this. We have provided some basic information for common build backends below.

I don't know if there's any way for mesonpy to fix this, but thought I'd open at least as a place to discuss as it doesn't seem to have come up before

@rgommers
Copy link
Contributor

rgommers commented Jan 2, 2024

Thanks for the issue and explanation @dstansby. It doesn't look like there's a good workaround. With spin using non-editable installs by default things do work, but IIRC Mypy is stopping to support running on installed packages, so that may not work forever either. Static type checkers seem to have severe limitations for anything that is not the legacy distutils/setuptools in-place build.

Reading microsoft/pylance-release#3473 and the linked thread, PyLance devs don't yet seem to understand the extent of this issue. scikit-build-core now also uses import hooks, as does setuptools by default. I think explaining the need for proper support of import hooks to the static typing folks is necessary here.

@rgommers rgommers added question Further information is requested external labels Jan 2, 2024
@eli-schwartz
Copy link
Member

For a bit of context, the issue here is that pth files are based on adding directories to sys.path, whereas import hooks are based on programmable logic that does whatever you want.

Meson-python doesn't need "programmable logic that does whatever you want", but it does need to be able to import this:

import foo       # uses foo/__init__.py
import foo._impl # uses builddir-release/foo/_impl.cpython-311-x86_64-linux-gnu.so

It is not conceptually possible for CPython to support this because you can't import the same top-level module from multiple different directories, unless you use namespace packages, which in turn means you can't have __init__.py and your API disappears.

In theory, type checkers can say "we don't care about this problem because we don't type check compiled extensions anyway, just stubs and pure python files".

Which is... technically true, as far as they are concerned, yes.

@rgommers
Copy link
Contributor

rgommers commented Jan 9, 2024

In theory, type checkers can say "we don't care about this problem because we don't type check compiled extensions anyway, just stubs and pure python files".

Hmm, that is a good point. Given that static checkers are big on "we don't import anything", it's unclear why they would care about this problem (unless there are generated .py/.pyi files) and could not simply be pointed at the source tree with some setting (it's typically . or ./pkgname).

Simple generated files like version.py could get a stub file that's checked in. For more complex generated Python files or sub files, there'd still be a problem - but that only applies to a small minority of packages.

@dnicolodi
Copy link
Member

Pylance does not want to support Python modules loaded via import hooks. meson-python editable install use a custom module finder to import the components of a package from multiple locations. The two things are not compatible. There isn't much meson-python can do to convince Pylance to support all Python features.

The Pylance documentation hints to the fact that Pylance cannot work on modules that rely on import hooks because it needs to load modules without executing them. I believe that this is because Pylance does not want to trigger any side effect that importing a module may have. However, I'm not sure that this is technically the case: I think there are public interfaces in the Python import machinery to actually get to the code of a module loaded with an import hook that do not require to execute the module. I haven't tried to implement this, thus I'm not sure this is feasible in all cases.

Anyhow, there is nothing to do on the meson-python side.

@dnicolodi dnicolodi added wontfix This will not be worked on and removed question Further information is requested external labels Jan 23, 2024
@eli-schwartz
Copy link
Member

I believe that this is because Pylance does not want to trigger any side effect that importing a module may have. However, I'm not sure that this is technically the case: I think there are public interfaces in the Python import machinery to actually get to the code of a module loaded with an import hook that do not require to execute the module.

The obvious caveat is that this requires executing the import hook as well as any modules the import hook imports.

@eli-schwartz
Copy link
Member

eli-schwartz commented Jan 23, 2024

Without executing any python code at all, a non-python type checker doesn't have to fork python at all, which is a speed improvement, not (just?) a security one. It's not obvious to me that security is the factor here.

@dnicolodi
Copy link
Member

Doesn't resolving typing annotations require executing Python code? I never looked at the details of how type checkers work, but my understanding is that with from __future__ import annotations the annotations are stored in a way that requires some form of evaluation to get to their value. If this is the case, running the import hook to get to the module spec would add very little additional work.

@eli-schwartz
Copy link
Member

No, it just requires lexing and parsing the grammar.

from __future__ import annotations causes CPython's builtin parser to avoid creating runtime code objects for annotations.

Pylance has its own implementation of the python grammar in JavaScript. It doesn't actually know what the code means -- it can't run a program -- but it can analyze the tree shape and object flow just fine and that's all it needs to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants