Repository navigation
Conversation
|
Hi @ysheikh2! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
456bea1 to
a110941
Compare
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
Sorry about the delayed response. I think this breaks a lot of assumptions we have: in the IDE, which environment do we pick when a file is open? I won't have enough time to review this today or reply to the original issue unfortuately. |
Hi, thanks for your feedback I've never come across this being an issue in any other type checker for python, with basedpyright, pyright, mypy it's always just worked for these monorepo setups and/or multi-root VSCode workspaces. A lot of my team hits this with our monorepos, as does the reporter of #3556, and it also comes up in #4987. The setup from #3556 is a VS Code workspace with two folders, each with its own interpreter: Both .venvs therefore have the same polars version. pyrefly picks a file's environment from where the file is, not from what imports it. To answer your question, an open file still uses its own folder's environment with this PR, so an open This is the same model the other type checkers use. pyright, basedpyright and ty use one environment per workspace folder, and every file a folder reaches, including a library installed as editable, is checked with that folder's environment. A file reached from two folders is analysed twice, once per folder. Eric Traut describes this in microsoft/pyright#4329 (comment). mypy uses one environment per run. This PR uses the importing environment instead of the workspace folder as the unit. |
A library with its own config and Python environment, installed editable into an app's environment, is checked with its own config in file mode and the language server. Its third-party imports then resolve in its own environment, so the app sees two copies of the same class (meta#3556). The test sets up an app and a library, each with its own site-package path containing a `dep` module, and expects no errors. It fails today.
In file mode and the language server, each file gets the config of its location. A library installed editable into an app's environment, but with its own config and environment, then resolves its imports in its own environment, and the app sees two copies of the same package (meta#3556). Project mode does not have this problem, because it uses one config for every file. `SysInfo` now also carries the site-package path of the environment. It is part of the `Handle` key and every import passes it on, so a file reached from two environments becomes two modules. When a file lies in one of its handle's site-package directories and that environment differs from its location config's, `State::get_config` uses a cached copy of the config with the importer's site-package path, so its imports resolve to the same modules as the importer's. A file opened on its own keeps its own config. Bundled stubs never carry an environment, and the stdlib is still computed once per version and platform.
a110941 to
e72cda7
Compare
|
This pull request has been imported. If you are a Meta employee, you can view this in D128474304. (Because this pull request was imported automatically, there will not be any future comments.) |
Summary
In file mode and the language server, each file gets the config of its location. If a library with its own config and
.venvis installed editable into an app's.venv, the library's imports resolve in its own.venv. The app then sees two copies of the same package and reports "DataFrameis not assignable toDataFrame". Project mode is not affected.This PR checks such a library in the importer's environment:
SysInfonow carries the site-package path. It is already part of theHandlekey and is passed on by every import.State::get_configuses a copy of the config with the importer's site-package path.An open file keeps its own folder's environment, so a library file opened on its own is checked as today. A library imported from two environments is checked once per environment. pyright, basedpyright and ty use the same model, with the workspace folder as the unit.
One open question for maintainers is whether the environment should live in
SysInfoor in a separateHandlefield.Fixes #3556
Prepared with an AI assistant (Claude) and reviewed by the author.
Test Plan
test_library_imported_from_another_environment: fails before the change, passes after.cargo fmt -- --check,cargo clippy --release,cargo test --profile bench.app+lib, and root +app) andpyrefly check main.py: 1 error before, 0 after. Project mode: 0 errors before and after.