pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.19k stars 613 forks source link

mypy dependencies are not used when running the check goal #21114

Open gcbirzan-plutoflume opened 1 week ago

gcbirzan-plutoflume commented 1 week ago

Describe the bug The documentation says that including type stubs in the mypy custom lockfile will cause them to be used when running check, but that's not true. This means that there's no way to emulate the extra_type_stubs behaviour in new pants, since the solution involves adding the stubs to the sources' dependencies.

Pants version 2.21, but this behaviour is also present in at least 2.20.

Additional info I have a repo that reproduces this problem: https://github.com/gcbirzan-plutoflume/pants-test

When I run check:

$ pants check ::
15:03:08.37 [WARN] No `/Users/cbirzan/PycharmProjects/pants-test/.python-version` found in the build root, but <PYENV_LOCAL> was set in `[python-bootstrap].search_path`.
15:03:09.35 [INFO] Completed: Building 1 requirement for requirements.pex from the test.lock resolve: requests==2.32.2
15:03:10.31 [INFO] Completed: Building 2 requirements for mypy.pex from the pants-mypy.lockfile resolve: mypy==1.5.1, types-requests==2.32.0.20240622
15:03:11.44 [INFO] Completed: Building requirements_venv.pex
15:03:13.91 [ERROR] Completed: Typecheck using MyPy - mypy - mypy failed (exit code 1).
foo.py:1: error: Library stubs not installed for "requests"  [import]
foo.py:1: note: Hint: "python3 -m pip install types-requests"
foo.py:1: note: (or run "mypy --install-types" to install all missing stub packages)
foo.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

✕ mypy failed.
krishnan-chandra commented 4 days ago

Hey, thanks for reporting this issue - I'm able to reproduce this issue using your example repo, so it definitely seems like the docs need some updates at least. I'll try to do a bit more digging to find a root cause.

gcbirzan commented 4 days ago

From what I saw, in older versions, the extra stubs were installed in requirements_venv, but the mypy venv isn't actually used for finding third party packages and their types, just what mypy runs in.

Without adding a similar feature to extra_type_stubs, I don't really see a simple way to fix this. I guess the mypy requirements can be installed in the requirements_venv, but that can cause conflicts.

krishnan-chandra commented 3 days ago

It looks like the docs are incorrect, based on an older comment here: https://github.com/pantsbuild/pants/issues/20259#issuecomment-1962236113

The type stubs would have to be present in the same resolve as the code being checked, which is what causes the issue.

gcbirzan commented 3 days ago

Okay, but then, maybe, the extra_type_stubs feature should be brought back? Even if they live in the code resolve, this means you don't have to add them to your prod code.

gcbirzan commented 3 days ago

Plus, it removes the need to add stubs to every piece of code that you want to check. Maybe it's not ideal, but I'm less bothered about mypy random deps, than for the whole codebase.

krishnan-chandra commented 3 days ago

Yeah, I agree with that - I wasn't around for when it was deprecated so I can't speak to the decision-making process there. @benjyw any thoughts since you were around for the last go-round?

benjyw commented 3 days ago

It's all a haze now... but I think it's laborious to require type stubs to be sequestered into extra_type_stubs, compared to the pip standard of jamming everything into the same requirements.txt. If Pants isn't smart enough to omit type stubs from contexts where they aren't needed then we should make it so.