DetachHead / basedpyright

pyright fork with various type checking improvements, improved vscode support and pylance features built into the language server
https://docs.basedpyright.com
Other
667 stars 14 forks source link

import resolution for generic virtual environments #365

Open gennaro-tedesco opened 2 months ago

gennaro-tedesco commented 2 months ago

First of all, thank you for the awesome project!

I am trying to come to terms with resolution of package imports within virtual environments (which I often found difficult in pyright as well): in particular I am referring to the import resolution section in the docs. Notice I have read https://github.com/DetachHead/basedpyright/issues/158 and https://github.com/DetachHead/basedpyright/issues/214 as well.

Description

I am using poetry to manage virtual environments: this means that virtual environments are generally created in some local paths ~/Library/pypoetry/virtualenvs/<projectname>-abc124... (or similar), with python packages locally installed are referred therein.

While running the project within the activated virtual environment, basedpyright always shows the diagnostic "Import could not be resolved" despite explicitly indicating the aforementioned path as per the documentation:

If a venv name is specified along with a python.venvPath setting (or a --venvpath command-line argument), it appends the venv name to the specified venv path...

The documentation moreover mentions

Use the python.pythonPath setting. This setting is defined by the VS Code Python extension and can be configured using the Python extension’s environment picker interface.

which is a little unclear to me if you are not using VS Code (the pythonPath is not a variable that even exists or is/must be explicitly set).

Attempts

[tool.basedpyright]
typeCheckingMode = "standard"
venvPath = "~/Library/pypoetry/virtualenvs/"  (or similar explicit path, or similar grammars)

What is the recommended way to specify paths to virtual environments for package resolution? (Notice I am referring to package imports and not relative imports from other modules, which work fine instead).

DetachHead commented 2 months ago

in my experience virtualenvs just work by default in pyright, so i've left the venvPath setting and its documentation unchanged from upstream as i'm not really familiar with it.

could you give a bit more info about how exactly you're running basedpyright? are you installing it outside of your venv and if so, how come?

gennaro-tedesco commented 2 months ago

Thank you for the prompt reply!

could you give a bit more info about how exactly you're running basedpyright? are you installing it outside of your venv and if so, how come?

I have installed it globally as python library via pip (or rather pipx), hence in that respect yes "outside" the virtual environment. Must it be installed within in (i. e. as a requirement package of a project within its environment)? I would find it a little strange (since then you'll have many basedpyright for each virtual environment rather than one only, which will be configured to "see" the different virtual environments per project).

DetachHead commented 1 month ago

Must it be installed within in (i. e. as a requirement package of a project within its environment)?

it's not a requirement, but it's what i've always done and i don't really understand why you wouldn't want to do that. making basedpyright a dev dependency of your project allows you to pin the version used by your IDE and your CI, which will prevent new errors or other changes in behavior from randomly appearing unexpectedly. this was my main motivation for creating basedpyright in the first place. i discuss the issue in more detail here.

as i said there, i recognize many people only use (based)pyright for its language server features and completely disable its type checking. i guess i could see why you'd want to just have it installed globally in that case? but i would highly recommend enabling the type checking and installing it to your venv instead.

all of that said, one of my goals is to maintain backwards compatibility with pyright so that users who don't agree with all of our decisions can simply disable the features they don't like. so this venvPath option should still work and i have no plans to remove it. i've just never personally used it before.

Attempts

[tool.basedpyright]
typeCheckingMode = "standard"
venvPath = "~/Library/pypoetry/virtualenvs/"  (or similar explicit path, or similar grammars)

What is the recommended way to specify paths to virtual environments for package resolution? (Notice I am referring to package imports and not relative imports from other modules, which work fine instead).

is the issue that poetry generates a random folder name for each of its venvs? if so, i think the solution here is to use an in-project venv, then set venvPath to ".venv"

i also want to take this moment to shill pdm as an alternative to poetry. in my experience it's far more stable and most importantly, unlike poetry, the maintainers didn't make the install script randomly fail in the CI 5% of the time. (even though this change was reverted before it was released, this shows that the maintainers are not trustworthy)

gennaro-tedesco commented 1 month ago

it's not a requirement, but it's what i've always done and i don't really understand why you wouldn't want to do that.

because doing so runs into the problem indicated here, namely the text editors (neovim in this case, but it would be the same for others) look for the general executable into ~/.local/bin (or equivalent); virtual environments are disposable and generated on the fly, and there is no consistent way to specify their generalities to editor configurations (unless you have one configuration per virtual environment).

if so, i think the solution here is to use an in-project venv, then set venvPath to ".venv"

This unfortunately doesn't work and hasn't worked in pyright either unless one explicitly fully qualifies the precise path to the disposable virtual environment, which again defies a little the purpose of configuration per sé.

i also want to take this moment to shill pdm as an alternative to poetry.

Sure, but this issue isn't exclusive to poetry, the server fails to recognise virtual environments created by other dependency managers too: pipenv and pdm too.

Generally speaking my question can be re-formulated as follows: is it possible to have a minimally reproducible example (say in the docs or README) of how to have basedpyright working with general virtual environments? I know the docs address to the .venv solution of pyright, but that has (infamously) never worked, with the proposed solution being to explicitly specify the full name of the disposable environment all the times (which I believe all agree is unusable).

P. S. Please don't get me wrong, this project is great and I appreciate the immense effort in its development! The handling of virtual environments however has kept many away from pyright originally already (most other language servers do recognise the virtual environments automatically), I have been using others successfully so far but wanted to switch to basedpyright given the great improvements it's bringing - which is why I am still a little puzzled that virtual environments being such a basic and common python development feature, one still needs to go to great lengths to have the lsp to work with them "out of the box".

DetachHead commented 1 month ago

This unfortunately doesn't work and hasn't worked in pyright either unless one explicitly fully qualifies the precise path to the disposable virtual environment, which again defies a little the purpose of configuration per sé.

oh wow, that's really stupid. we should definitely fix that

miversen33 commented 1 month ago

as i said there, i recognize many people only use (based)pyright for its language server features and completely disable its type checking. i guess i could see why you'd want to just have it installed globally in that case? but i would highly recommend enabling the type checking and installing it to your venv instead.

It's worth calling out that while you make good points here, most editors don't care. Neovim (mason specifically) for example downloads a single LSP and uses it for all matching filetypes. Vscode will do the same. While it is nice to have your LSP as part of your dev requirements, I would hesitate to consider it a requirement to get virtualenvs functional.

DetachHead commented 1 month ago

Neovim (mason specifically) for example downloads a single LSP and uses it for all matching filetypes. Vscode will do the same.

the basedpyright vscode extension defaults to using the version installed in the current project's venv, and will only fallback to using the version bundled with the extension if you don't have it installed already. i'm not sure how the extensions for other IDEs work since they were contributed by others and i personally only use vscode, but ideally they would all behave the same way.

While it is nice to have your LSP as part of your dev requirements, I would hesitate to consider it a requirement to get virtualenvs functional.

i agree. as i said in my previous comment, it seems really dumb that the venvPath needs to be absolute, which arguably makes the setting completely useless in its current state. i intend to fix this

gennaro-tedesco commented 1 month ago

i'm not sure how the extensions for other IDEs work

my experience with terminal text editors is that they default to the global LSP executable because by definition their configurations are global, whereas IDEs tend to have mechanisms to "automatically activate" virtual environments within themselves (although even here I am not sure that this activation is automatic and must not be explicitly invoked by the user instead - I remember using pyright with PyCharm and somehow I could never make it work).

Generally speaking I would say that if one envisions for this project to be adopted in industrial settings (where many people contribute to the same code) then there ought to be an easi(er) way to indicate that the LSP must look for references and packages into a virtual environment first (if present) - the rationale being that if I use it myself only, then I could configure full paths and all the rest on my local machine, but if I work with my team on a shared github repository then we cannot just commit into the pyproject.toml each single independent configuration that points to our local paths.