bazel-contrib/bazel-mypy-integration

Stubs from pypi are not found by mypy

fwingerter-Ocient opened this issue · 22 comments

fwingerter-Ocient commented

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?

rogerhub commented

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.

  • 👍3
  • 👀1
mbkroese commented

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.

alexeagle commented

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.

mbkroese commented

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 commented

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?

alexeagle commented

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.

mbkroese commented

Ok, makes sense, and sounds good to me!

alexeagle commented

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.

  • 👍2
mbkroese commented

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.

juanique commented

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.

  • 👍1
ph03 commented

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 ^^)

caseyduquettesc commented

Reading through the comments, it's not clear to me why this regressed. If I use an older release, I do not experience any of the errors described by others, with any mypy version.

git_repository(
    name = "mypy_integration",
    commit = "c1193a230e3151b89d2e9ed05b986da34075c280",
    remote = "https://github.com/thundergolfer/bazel-mypy-integration",
    shallow_since = "1639112081 +1100",
)

If I try a newer release, such as

git_repository(
    name = "mypy_integration",
    commit = "285d2a0d31c42eb273a0287195d36c6908d6e838",
    remote = "https://github.com/bazel-contrib/bazel-mypy-integration",
    shallow_since = "1665756151 -0700",
)

I begin experiencing the "Cannot find implementation or library stub for module" errors.

robin-wayve commented

@ph03 The issue I linked is what got fixed: mypy changed to search sys.path as well as site-packages for library stubs (and Bazel / rules_python puts everything into sys.path).

I can't speak for all of this integrating with versions of mypy_integration since I am not using it, but did find the monkey-patch suggestion useful for getting Bazel python targets to find library stubs.

kevinaud commented

Any updates on this? I'm using mypy version 0.971 and bazel-mypy-integration 0.4.0 but mypy is still complaining that it cannot find my external libraries.

jscheel commented

I'm experiencing this as well. @alexeagle given that a year has passed, do you think rules_py is capable of helping with this now?

alexeagle commented

We're discussing it again now, but it would still help to have a funding source for this repo. https://opencollective.com/bazel-rules-authors-sig currently owes more than it has.

alexmirrington commented

I've been working on a separate project based on this one since we haven't seen a lot of activity in this repo lately, and added support for PEP-561 stub packages over here: alexmirrington/rules_mypy#9
It also has support for generating and type-checking generated .pyi files from protobufs and gRPC stubs if that sounds like something you might want as well 🎉

alexeagle commented

Hey @alexmirrington I finally have some client funding for working on mypy again. I'm interested to study what you've done and collaborate on this :)

  • 🎉3
alexmirrington commented

@alexeagle Great to hear! The biggest learning I've found from playing around with PEP-561 stub packages is that it's easiest to generate a virtual environment and use the --python-executable mypy flag to discover the stubs instead of trying to symlink stubs using starlark.

I've had success with this using rules_pyvenv, but those rules will only create the venv on run rather than build, and also break out of the runfiles tree which is useful for intellisense but not really what we want for mypy aspects.

I'm currently working on creating some venv rules to replace the starlark I wrote to symlink stubs, would be keen to see if you have any other ideas

alexeagle commented

rules_py creates a venv for every binary/test, I think it could probably create one for libraries as well. See aspect-build/rules_py#235 and https://github.com/aspect-build/rules_py/blob/main/docs/rules.md#py_venv

chrisirhc commented

Just to add here, one way to make stubs available to mypy is to make it available to mypy binary as deps:

# in /toolings/typing/BUILD

load("@rules_python//python/entry_points:py_console_script_binary.bzl", "py_console_script_binary")
load("@pip…", "requirement") # your pip setup

py_console_script_binary(
    name = "mypy",
    pkg = "@pip…//mypy", # your pip setup
    deps = [
        requirement("grpc-stubs"),
        requirement("pytest"),
    ],
)

Then provide this as the mypy via the .bazelrc

build --@mypy_integration//:mypy=//toolings/typing:mypy

This wasn't obvious to me so it might help someone else. Perhaps it could be an example or documented use case.

  • 👍2