bazel-mypy-integration icon indicating copy to clipboard operation
bazel-mypy-integration copied to clipboard

Stubs from pypi are not found by mypy

Open fwingerter-Ocient opened this issue 2 years ago β€’ 21 comments

I've tried several different ways of referencing packages like types-python-dateutil from PyPI and I can't get any of them to actually provide the type stubs to mypy using bazel-mypy-integration.

Here's a repo where I've demonstrated the approaches I've tried.

  1. Make the pypi target as returned by requirement() a dependency of the py_binary target passed to mypy_test.
  2. Define a mypy_stubs target manually pointing at the .pyi files inside the pypi target.
  3. Define a mypy_stubs target pointing at the pypi target as returned by requirement(). This one causes bazel errors because the py_library rule internal to the pip_install workspace rule does not include .pyi files in its srcs (only in data), as I understand it.

When running bazel test --test_output=all //:uses_deps_mypy, I see the following output:

INFO: From Testing //:uses_deps_mypy:
==================== Test output for //:uses_deps_mypy:
INFO: Analyzed target //:uses_deps_mypy (33 packages loaded, 1623 targets configured).
INFO: Found 1 test target...
FAIL: //:uses_deps_mypy
uses-deps.py:1: error: Library stubs not installed for "dateutil" (or incompatible with Python 3.8)
uses-deps.py:1: note: Hint: "python3 -m pip install types-python-dateutil"
uses-deps.py:1: note: (or run "mypy --install-types" to install all missing stub packages)
uses-deps.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)
================================================================================
Target //:uses_deps_mypy up-to-date:
  bazel-bin/uses_deps_mypy
INFO: Elapsed time: 3.973s, Critical Path: 3.21s
INFO: 7 processes: 5 internal, 2 processwrapper-sandbox.
INFO: Build completed, 1 test FAILED, 7 total actions
//:uses_deps_mypy                                                        FAILED in 2.6s

Interestingly, if I run mypy manually using the stubs as downloaded by bazel from pypi, I get the same error:

$ MYPYPATH=~/.cache/bazel/_bazel_fwingerter/13dabd0983aec3a797d4285d705dd267/external/mypy_stubs/pypi__types_python_dateutil/ mypy uses-deps.py
uses-deps.py:1: error: Library stubs not installed for "dateutil" (or incompatible with Python 3.8)
uses-deps.py:1: note: Hint: "python3 -m pip install types-python-dateutil"
uses-deps.py:1: note: (or run "mypy --install-types" to install all missing stub packages)
uses-deps.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
uses-deps.py:2: error: Library stubs not installed for "dateutil.parser" (or incompatible with Python 3.8)
Found 2 errors in 1 file (checked 1 source file)

But if I rename the directory to dateutil, it works (obviously this is not sane as one shouldn't muck around in the bazel cache, but it helps demonstrate that mypy and the bazel plumbing do not seem to agree on directory structure):

$ cd ~/.cache/bazel/_bazel_fwingerter/13dabd0983aec3a797d4285d705dd267/external/mypy_stubs/pypi__types_python_dateutil
$ cp -ar dateutil-stubs dateutil
$ cd -
$ MYPYPATH=~/.cache/bazel/_bazel_fwingerter/13dabd0983aec3a797d4285d705dd267/external/mypy_stubs/pypi__types_python_dateutil/ mypy uses-deps.py
Success: no issues found in 1 source file

What is the intended way to use stub libraries from PyPI with bazel-mypy-integration?

fwingerter-Ocient avatar Jul 27 '21 16:07 fwingerter-Ocient

I ran into this issue myself and monkey-patched a hacky solution. Here's what I discovered:

  1. Each of the types-* PyPI packages contains a top-level directory named "...-stubs" (e.g. certifi-stubs for the "types-certifi" package).
  2. The mypy module finder looks for these ...-stubs directories, but it will only search directories in the "package_path".
  3. The "package_path" basically comes from site.getsitepackages() + [site.getusersitepackages()]. You can follow the call sites from module finder to the mypy pyinfo module.
  4. When Bazel builds PyPI dependencies into a binary, it just adds them to sys.path. This allows code to run import certifi and find the certifi package files. However, this is different from typical Python setups, in which you have dist packages, site packages, and maybe a user packages directory. There is no site packages directory for the Bazel PyPI dependencies, so mypy's package_path does not find them.

Mypy also offers a MYPYPATH, but it does not use the "...-stubs" layout, so we can't just stick all of the Bazel PyPI directories into the MYPYPATH (I tried that initially).

So, here's my hacky solution:

step 1: monkey patch mypy modulefinder

I grabbed all the pypi__ paths from sys.path, then I added them as new "site packages".

diff --git a/mypy/main.py b/mypy/main.py
index 04442ad..c81e0ed 100644
--- a/mypy/main.py
+++ b/mypy/main.py
@@ -1,7 +1,20 @@
 
 
+import os
 import sys
+
+import mypy.modulefinder
 from mypy.main import main
 
 if __name__ == '__main__':
+    additional_package_paths = [p for p in sys.path if 'pypi__' in p]
+    original_get_site_packages_dirs = mypy.modulefinder.get_site_packages_dirs
+
+    def get_site_packages_dirs(*args, **kwargs):
+      egg_dirs, site_packages = original_get_site_packages_dirs(*args, **kwargs)
+      site_packages += tuple(additional_package_paths)
+      return egg_dirs, site_packages
+
+    mypy.modulefinder.get_site_packages_dirs = get_site_packages_dirs
+
     main(None, sys.stdout, sys.stderr)

step 2: add type packages as dependencies

The types-... packages need to be available to the mypy binary when it does the type checking, so I just added them as dependencies:

diff --git a/mypy/BUILD b/mypy/BUILD
index c8f843d..71558e3 100644
--- a/mypy/BUILD
+++ b/mypy/BUILD
@@ -12,6 +12,13 @@ py_binary(
         requirement("typing_extensions"),
         requirement("mypy_extensions"),
         requirement("typed_ast"),
+
+        # Custom dependencies.
+        requirement("types-certifi"),
+        requirement("types-protobuf"),
+        requirement("types-requests"),
+        requirement("types-setuptools"),
+        requirement("types-six"),
     ],
 )

These also need to be added to your mypy_version.txt file, so that they get added to mypy_integration_pip_deps. Mine looks like this now:

mypy==0.910

types-certifi
types-protobuf==3.17.4
types-requests
types-setuptools
types-six==1.16.0

I applied both of these patches in my workspace file like so:

mypy_integration_version = "0.2.0"  # Latest @ 26th June 2021

http_archive(
    name = "mypy_integration",
    sha256 = "621df076709dc72809add1f5fe187b213fee5f9b92e39eb33851ab13487bd67d",
    strip_prefix = "bazel-mypy-integration-{version}".format(version = mypy_integration_version),
    urls = [
        "https://github.com/thundergolfer/bazel-mypy-integration/archive/refs/tags/{version}.tar.gz".format(version = mypy_integration_version),
    ],
    patch_args = ["-p1"],
    patches = [
        "@//:data/patches/mypy_integration/0004-stubs.patch",
        "@//:data/patches/mypy_integration/0005-site_packages.patch",
    ],
)

Of course, I think it would be ideal to have native support for this sort of thing (or let me know if I'm doing something wrong), but this solution works for me in the short-term.

rogerhub avatar Aug 22 '21 16:08 rogerhub

I'm also affected by this issue. I would have thought the deps section of the mypy_test rule would have worked, but it doesn't.

mbkroese avatar Apr 25 '22 14:04 mbkroese

At a high level, the issue is that Bazel creates a non-idiomatic layout of python files. It then provides a "stub" in py_binary which tries to correct for this layout by doing things like patching up the sys.path. However mypy expects a site-packages folder to exist, like pip would create. Bazel's stub is a leaky abstraction and causes incompatibilities like this.

So at a high level I think there are two solutions:

  1. Do even more patching, like @rogerhub illustrates we just need these stub packages to appear in site_packages at the point mypy reads from there. Either by monkey-patching mypy or the standard library it relies on to read from site_packages
  2. Lay out a python-idiomatic virtualenv in the bazel-out tree so that all tools just transparently work, using https://github.com/aspect-build/rules_py:

    We don't mess with the Python sys.path/$PYTHONPATH. Instead we use the standard site-packages folder layout produced by pip_install.

alexeagle avatar May 06 '22 12:05 alexeagle

Do even more patching, like @rogerhub illustrates we just need these stub packages to appear in site_packages at the point mypy reads from there.

This involves setting the PYTHONPATH (sys.path), if I understand you correctly.

I think there is a third solution, which is make the packages available via MYPYPATH as the original author has attempted. The problem with this current approach is:

  1. The symlinks for the *.pyi files are missing in my_deps/pypi__types_python_dateutil
  2. The folder inside pypi__types_python_dateutil needs to be renamed from dateutil-stubs to dateutil, as suggested by fwingerter-Ocient

This approach suffers from another problem, which is that the python packages installed on the system leak into the Bazel python setup and are available to the mypy command. This can be verified by simply pip-installing types-python-dateutil with python3 on the host system.

mbkroese avatar May 09 '22 07:05 mbkroese

Lay out a python-idiomatic virtualenv in the bazel-out tree so that all tools just transparently work, using https://github.com/aspect-build/rules_py

I think this is a great initiative but since this still seems experimental I think it would be useful to also support users that use the more standard bazelbuild/rules_python?

mbkroese avatar May 09 '22 07:05 mbkroese

rules_py would be in addition to rules_python, not replacing it. In that model, it's just these type-checking actions which would run inside a standard virtualenv created by those rules, but all other actions/tests would be unaffected.

alexeagle avatar May 09 '22 14:05 alexeagle

Ok, makes sense, and sounds good to me!

mbkroese avatar May 09 '22 15:05 mbkroese

Dropping some notes here about my option 2 above:

  • rules_py assumes the use of a hermetic, downloaded Python interpreter. However the indygreg Mac arm64 interpreter is built assuming a fixed XCode installation path, which I don't have installed. Switching this repo to use the hermetic interpreter causes the C compile of the typed-ast package to fail to locate headers. I could install the matching XCode too, but I think that would mean users have to do that as well. My system python does locate all the headers needed to install that package.
  • newer mypy (starting at 0.900) no longer depends on the typed-ast package. However the command line API seems different, it fails to parse our flags, thinking --bazel or --package-path=. should be file paths. Anyway we don't want to force users to change their mypy version.
  • even if I could fix those, the next issue would be that rules_py installs a virtualenv for the tool being run, which is to say the static dependencies declared on the py_binary(name = mypy) rule. However the type stubs are provided by users when the aspect/test invokes that mypy binary, and those stubs won't be installed in the virtualenv. rules_py does stitch them into the .pth file, so the regular importlib works. But as observed earlier in this issue, mypy itself doesn't use importlib for resolutions, rather it assumes pathing in the site-packages folder.

I think the conclusion here is that rules_py isn't ready for this use case, and we'd need still need some monkey-patch hacks to get it to work.

alexeagle avatar May 11 '22 14:05 alexeagle

Looking at the runfiles, we have the following structure for //third_party:uses_deps_mypy:

➜  uses_deps_mypy.runfiles tree -L 1
.
β”œβ”€β”€ MANIFEST
β”œβ”€β”€ __init__.py
β”œβ”€β”€ bazel_tools
β”œβ”€β”€ examples
β”œβ”€β”€ my_deps
β”œβ”€β”€ mypy_integration
β”œβ”€β”€ mypy_integration_config
└── mypy_integration_pip_deps

Looking inside my_deps, I can find the types_dateutil there. Do we want to keep this runfiles structure?

If yes, there is a problem that the .pyi files are not there. From some initial investigation it looks like they are not included in this line:

dep[PyInfo].transitive_sources

which is at line 75 of mypy.bzl.

mbkroese avatar May 20 '22 15:05 mbkroese

This was fixed in mypy 0.971 πŸŽ‰ https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

https://github.com/python/mypy/pull/11143

robin-wayve avatar Aug 04 '22 09:08 robin-wayve

When I use mypy version 0.971, all my mypy_test targets start failing with:

/workdir/bazel-output-base/sandbox/processwrapper-sandbox/119/execroot/repo/bazel-out/k8-fastbuild/path/to/target_mypy.runfiles is in the MYPYPATH. Please remove it.

See example invocation: https://app.buildbuddy.io/invocation/82dd2ab8-d1de-4cf5-a1ca-7cd350927bf7.

I'm currently using 0.910 and they work fine.

juanique avatar Aug 14 '22 21:08 juanique

This was fixed in mypy 0.971 tada https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html

python/mypy#11143

@robin-wayve Do you mind elaborate on what exactly got fixed by mypy-0.971, as using this version along with bazel-mypy-integration-0.4.0 I still run into issues like

ERROR: XXX/src/py/hello/BUILD.bazel:20:12: Type-checking //src/py/hello:test failed: (Exit 1): test_mypy_exe failed: error executing command bazel-out/k8-opt/bin/src/py/hello/test_mypy_exe

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
bazel/pytest/pytest_wrapper.py:2: error: Cannot find implementation or library stub for module named "pytest"
src/py/hello/world.py:3: error: Cannot find implementation or library stub for module named "numpy"
src/py/hello/world.py:3: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 2 errors in 2 files (checked 3 source files)

What's the latest suggestion to make this work (I didn't try the monkey-patch approach yet as I don't want to push this to everyone my the project ^^)

ph03 avatar Oct 11 '22 07:10 ph03