Implement a polished v2 MyPy experience

[Eric-Arellano]

• Resolve 3rd party requirements, including 3rd party plugins/stubs.
• Support first-party plugins.
• Support .pyi files.
• Don't strip source roots in the final output. Fixed by Preserve source roots in MyPy and Pylint output #10159.
• Figure out the story on interpreter constraints.
  • Partition based on interpreter constraints.
• Documentation.

No longer in scope:

• Handle namespace packages declared via setuptools and PEP 420. MyPy doesn't support this well to begin with Redesign import handling  python/mypy#8584
• Performance similar to MyPy without Pants. We do want to do this, but it's tricky and requires new tools for handling external caches.

[stuhood]

We should probably break out the 3rdparty aspect of this and ensure that it is done by 2.0.x, but perhaps the rest could be deferred till after 2.0.x?

[danieljanes]

Since this issue mentions a polished mypy experience, can I suggest support for https://github.com/dropbox/mypy-protobuf ?

mypy-protobuf automatically generates type defs for protoc-generated Python files, which improves the mypy experience quite a bit. Or should this better be filed under some protoc-related topic?

[Eric-Arellano]

Great suggestion. mypy-protobuf would fall under the "enable plugins and type stubs" part. I will explicitly list it, along with django-stubs, as libraries that we need to properly support.

[Eric-Arellano]

The main open question is whether we want a dedicated python_type_stubs target, or to allow python_library, python_tests, and python_binary to use .pyi files too.

A user may write .pyi stubs for their own code, including their own tests and their python_binary sources. This is common if you use Python 2 and prefer type stubs to type comments. They also might write stubs for third-party code, without any corresponding first-party implementation.

--

We need to teach dependency inference to accommodate type stubs, which means it needs to infer deps on both the original implementation and also the stubs. However, dep inference should continue to no-op if >2 implementations are detected.

We could see if it's a stub by inspecting if the file address ends in .pyi, which is hacky but works. A more structured approach is to use the Target API to look for PythonTypeStubsSources.

--

A major downside of a dedicated python_type_stubs target is that it means it will be likely the user only has one single python_type_stubs target per folder, as the default source globs would be *.pyi. If tests and source files are colocated, their stubs will belong to the same target, whereas normally we try to separate out library from test code.

Thanks to dependency inference operating at the file level—along with explicit file addresses—this isn't a big deal usually. Even if python_type_stubs refers to both lib.pyi and lib_test.pyi, Pants will infer dependencies on just the distinct files.

However, this breaks with explicit dependencies on the target, such as depending on :stubs. Why do we care about this? python_distribution. A python_distribution target cannot use dependency inference; it must explicitly declare its dependencies. Python distributions should include their type stubs with them (PEP 561). If a user declares a dependency on :stubs, they may end up with stubs for their test files, which they probably did not intend to do; usually, you leave tests out of your distribution.

--

However, updating python_library, python_tests, and python_binary isn't perfect, either.

Now that Pants always operates on a file-by-file basis, such as running tests one-file-at-a-time (batching support pending), including type stubs in a python_tests target would naively result in trying to run Pytest on the type stubs, which is an error. We could special-case pytest_runner.py to skip .pyi files, just like it skips conftest.py. This is doable, but a bit hacky.

python_binary currently only allows for 0 or 1 source file. We would presumably want the type stub to also live with its implementation in the same python_binary target, so we would need to have complex logic that allows for 0 or 1 source files, or 2 if one of them is a stub.

Taking a step back, conceptually, I'm not sure that type stubs fit the bill of python_binary or python_tests targets, which are "metadata for a binary" and "metadata for test files", respectively.

--

I don't think we should consider python_type_stubs_library, python_type_stubs_tests, etc. Way too clunky.

[stuhood]

If tests and source files are colocated, their stubs will belong to the same target, whereas normally we try to separate out library from test code.

In general, tests shouldn't have stubs, since they shouldn't have dependents? Test-support libraries will still be libraries.

A python_distribution target cannot use dependency inference; it must explicitly declare its dependencies.

Given that a python_tests target shouldn't have dependents (and thus shouldn't need stubs) this seems fine. Everything other than the top-most target included in the python_distribution should be inferred, and it might even be reasonable for a a stubs module to always depend on its own module, and vice-versa.

python_binary currently only allows for 0 or 1 source file. We would presumably want the type stub to also live with its implementation in the same python_binary target, so we would need to have complex logic that allows for 0 or 1 source files, or 2 if one of them is a stub.

You shouldn't need stubs in the python_binary case, because it's a "root" of the graph (from a PYTHONPATH perspective), similar to python_tests.

---

Another consideration is that if pyi files are included in the underlying target they would be included in archives and test environments, where nothing would consume them. Placing them in a separate target type makes the most sense to me.

[Eric-Arellano]

In general, tests shouldn't have stubs, since they shouldn't have dependents? Test-support libraries will still be libraries.

You sometimes use .pyi files for your own code if you have to support Python 2, and prefer using .pyi syntax rather than type comments, given that normal type hints aren't allowed with Py2. I'm referring to this case, that you wrote lib_test.pyi for your own code.

[benjyw]

I think treating *.pyi files like python sources makes sense (e.g., in default globs). If people don't want their test stubs pulled into non-test artifacts they can split them into a separate _test.pyi stub file. The pytest runner can special-case test discovery to ignore those files. This seems simple and straightforward. Having new target types seems like overkill.

I think there's no need to do anything about python_binary. It can put the type stubs of its source into a python_library dependency. The only reason we support a sources= field on python_binary is as a tiny bit of sugar to avoid having to specify an entry_point (frankly I'd be fine if we dropped that entirely, but that's just me). No need to make that overcomplicated.

[stuhood]

The pytest runner can special-case test discovery to ignore those files. This seems simple and straightforward. Having new target types seems like overkill.

They will also be skipped to production though, which is probably not intended. So there is more than just one place to special case them: all consumers of that target type other than mypy would want to filter them.

[benjyw]

Could add a type_stubs field to python_library and python_tests? Like a sources field but just for type stubs, with a *.pyi default globs?

So basically everything acts the same as today, except for mypy, which knows to look at that field. The downside is people will have to know to use this field, instead of sources, to manually add stubs. But I'm comfortable with that.

[Eric-Arellano]

Could add a type_stubs field? Like a sources field but just for type stubs, with a *.pyi default globs?

Possibly, I've been pondering it all day and talked it over with Cristian, who does have experience with type stubs, both for bindings and for first-party code.

It gets a little tricky that Cristian and I agree that we do want to have autoformatters like Black and Isort run on .pyi files. We also very likely want linters like Pylint and Flake8 to run on them, although I need to check the behavior of what happens if you have a .py and .pyi file at the same time.

Whatever implementation we go with, I think that we're going to want to add tests to all 6 linters, MyPy, Pytest, and setup-py that they do the right thing.

They will also be skipped to production though, which is probably not intended

I don't follow what you mean. At least for setup-py, it's very important that the .pyi files are still included.

--

On Friday, I was advocating that we don't support type stubs for your own first-party code. We only support type stubs used as bindings for third party code.

I'm still not sure the right decision. Cristian has a real-world use case of preferring .pyi files for source code files. He wrote an extension of SQLAlchemy, and it's more ergonomic to have the stubs than to try to inline the hints. He's been working on seeing if inline is indeed even possible, due to some issues with Python 3.6 and inheriting Generic (which was a metaclass) breaking SQLAlchemy.

--

If we do support type stubs for your own files, we have a tricky problem: how do we teach Pants dependency inference about the relationship between a .py file and its sibling .pyi file? Likely, they should be linked.

The type_stubs field might work around that. Although, it would interact weirdly with the "Pants always runs one-file-at-a-time" paradigm. Again, we want to run Black on .pyi files, and we do allow running Black one-file-at-a-time if you prefer. Now, we'd break that. You'd run one implementation file at-a-time, plus any arbitrary stub files in that same run.

[stuhood]

They will also be shipped to production though, which is probably not intended

I don't follow what you mean. At least for setup-py, it's very important that the .pyi files are still included.

PEXes, repls, tests, etc will not want them present.

[Eric-Arellano]

This is now all implemented :) Performance improvements are now tracked by #10864. #10497 tracks the MyPy Protobuf plugin, which is more of a Protobuf project than MyPy project.

[benjyw]

Rockin'!