Alternative fix for pyright CI issues #10121
Conversation
|
Oh yeah that works too. Didn't think of pointing pyright to a venv. It does feel cleaner than my suggestions. For #9989 you could (if you wanted) uninstall SQLAlchemy 2 from the venv. Just like we do for setuptools/distutils. |
Ooh, interesting idea! Feels more like a workaround than a fix though -- the fundamental issue is that we should probably be testing each third-party stubs package in isolation, like we do with mypy, and it wouldn't do anything to fix that. For me, it's just a question of whether we can tolerate the extra complexity that doing that would entail (I'm not sure I can! I really like how simple running pyright via a GitHub Action is!). |
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "$schema": "https://raw.githubusercontent.com/microsoft/pyright/main/packages/vscode-pyright/schemas/pyrightconfig.schema.json", | |||
| "typeshedPath": ".", | |||
| "venv": ".venv", | |||
There was a problem hiding this comment.
Does this mean that it isn't going to work for people who prefer to name their venv something else? I usually name it env. It might not be a problem for me, because I don't run pyright locally very often.
There was a problem hiding this comment.
Yes, I think it will mean that. FWIW, I also prefer env as a name for my local virtual environments (it's fewer characters to type), but went with .venv here since it's the name recommended by our CONTRIBUTING docs:
Line 59 in ae0c9f9
There was a problem hiding this comment.
.venv is also what's configured for flake8, gitignore, and isort.
mypy_test and regr_test use a temp directory anyway, but it's also using .venv
So it's pretty established that the standard for typeshed is to name the virtual environment folder .venv
There was a problem hiding this comment.
I would still prefer passing it in a command-line option instead of hard-coding it in the config.
There was a problem hiding this comment.
I would still prefer passing it in a command-line option instead of hard-coding it in the config.
So would I, but if I'm reading the docs for pyright correctly, I don't think that's an option
There was a problem hiding this comment.
Apparently pyright has a -v, --venv-path option whose purpose is exactly this. From the docs:
(3) This option is used in conjunction with configuration file, ... This allows a common config file to be checked in to the project and shared by everyone on the development team without making assumptions about the local paths to the venv directory on each developer’s computer.
There was a problem hiding this comment.
The venv-path option can be set via the command line (and I do in this PR, via pyright-action). But the venv-path option is meant to point to a directory in which there might be multiple different virtual environments, if I'm reading the pyright docs correctly, meaning it's useless to set the venv-path option on the command line unless you also set the venv option in the config file. The venv option is what tells pyright which venv it should select out of the ones it finds in the path specified by venv-path.
There was a problem hiding this comment.
See the more detailed docs here, which the command-line docs link to, and which dig into the distinction between the venvPath and venv settings: https://github.com/microsoft/pyright/blob/main/docs/configuration.md#main-configuration-options
(venvPath in the config file is equivalent to venv-path on the command line)
There was a problem hiding this comment.
Apparently there's no way to set this on the command line.
Helps with #10112 (an alternative fix to #10120). Install 3rd-party stub dependencies into an isolated environment, and use that for running pyright on typeshed, rather than installing the 3rd-party stub requirements into the same environment all our test requirements are installed in.
As with #10120, this doesn't fix the issue encountered in #9989 or #10090, but it does unblock #10058.