Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MyPy to Pants v2 #10132

Merged
merged 3 commits into from Jun 22, 2020
Merged

Conversation

Eric-Arellano
Copy link
Contributor

This is not a perfect implementation yet. See #10131 for remaining TODOs. But, this gives an initial implementation to build off of.

A key decision of this PR is to add MyPy to the lint goal, rather than a new typecheck goal. Users shared feedback that they prefer this, as it's neat to run all your linters in parallel. Technically, MyPy is a linter, only a supercharged one.

To activate, add pants.backend.python.lint.mypy to backend_packages2.

[ci skip-rust-tests]
[ci skip-jvm-tests]
It resulted in every file in the result being prefixed by `src`. Not good!
[ci skip-rust-tests]
[ci skip-jvm-tests]
# Delete this line to force CI to run Clippy and the Rust tests.
[ci skip-rust-tests]
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

A key decision of this PR is to add MyPy to the lint goal, rather than a new typecheck goal. Users shared feedback that they prefer this, as it's neat to run all your linters in parallel. Technically, MyPy is a linter, only a supercharged one.

Not a blocker, but one other possibility that would be interesting to explore before 2.0 would be to allow for some parallelism between goal rules for the portions that don't have sideeffects. That would allow for something like ./pants check lint to run both the check and lint goals in parallel, but to actually complete the goals (ie, render their outputs, affect the filesystem) in a specified order.

I'm pretty sure that it can be made to work... just a question of whether it would be desirable to expose this interface. It would have advantages for other workflows as well, such as ./pants test lint, etc.

@Eric-Arellano
Copy link
Contributor Author

Eric-Arellano commented Jun 22, 2020

but one other possibility

Interesting. For the sake of MyPy, users I think also liked the less typing of only lint rather than lint typecheck.

But I agree that that would be a neat feature. Doesn't seem blocked by 2.0 though - we would be improving parallelism, and not changing the final result.

@Eric-Arellano Eric-Arellano merged commit ac32a87 into pantsbuild:master Jun 22, 2020
@Eric-Arellano Eric-Arellano deleted the mypy branch June 22, 2020 22:58
@stuhood
Copy link
Sponsor Member

stuhood commented Jun 23, 2020

But I agree that that would be a neat feature. Doesn't seem blocked by 2.0 though - we would be improving parallelism, and not changing the final result.

The relation to 2.0 is just that moving mypy between goals is potentially costly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants