Offer more python linter tools (black, mypy, isort)

[svenevs]

Followup task from #17962. Python files developed there were originally being linted with the following:

#!/usr/bin/env python3

# This file is in tools/workspace/vtk/lint_me.py
#
# Dependencies:
#     pip install black flake8 flake8-import-order mypy
#
# Running it:
#     MYPYPATH="$PWD:$PWD/image" ./lint_me.py

import os
import subprocess
from pathlib import Path

def main():
    this_file_dir = Path(__file__).parent.absolute()
    py_files = [str(p) for p in this_file_dir.rglob("*.py")]

    subprocess.check_call(
        [
            "black",
            "--line-length",
            "80",
            *py_files,
        ]
    )

    mypy_env = os.environ.copy()
    mypy_path = f"{this_file_dir}:{this_file_dir / 'image'}"
    mypy_env["MYPYPATH"] = mypy_path
    subprocess.check_call(
        [
            "mypy",
            "--config-file",
            str(this_file_dir / "mypy.ini"),
            "--explicit-package-bases",
            str(this_file_dir),
        ],
        env=mypy_env,
    )

    subprocess.check_call(["flake8", *py_files])

if __name__ == "__main__":
    main()

and a config file

[mypy]

[mypy-lsb_release.*]
ignore_missing_imports = True

[mypy-bazel_tools.*]
ignore_missing_imports = True

Drake has mypy, we could consider adding to //tools/lint and adding a new lint check on the tools/workspace/vtk directory. mypy is still a tool that you have to coerce at times, but IME it's worth it. The example that came up is it would have helped me know I broke the linux codepath when refactoring on mac. There are certainly downsides to it, but at least for that code, it's setup and able to be type checked so I think we should continue to keep it that way if possible.

Drake already runs flake8, and I'll let the birds decide on black. I'd say there's no excuse not to use black if we also use clang-format :wink: But it's not already part of the drake dependencies.

There are other places (drake-ci) I would like to re-request these three tools for.

CC @jwnimmer-tri @BetsyMcPhail @mwoehlke-kitware as the people who will also have to interact with these.

[jwnimmer-tri]

See also #13571 for adding isort as a linter.

Also note that https://github.com/RobotLocomotion/drake-blender has some Bazel integration of both black and isort that we could backport into Drake.

I think the call to action here is to add all three tools as linter opt-ins, like so:

--- a/tools/lint/lint.bzl
+++ b/tools/lint/lint.bzl
@@ -14,6 +14,9 @@ def add_lint_tests(
         bazel_lint_extra_srcs = None,
         bazel_lint_exclude = None,
         enable_clang_format_lint = True,
+        enable_black_lint = False,
+        enable_isort_lint = False,
+        enable_mypy_lint = False,
         enable_install_lint = True,
         enable_library_lint = True):
     """For every rule in the BUILD file so far, and for all Bazel files in this

Possibly we should split each one (isort, black, mypy) into a dedicated issue number. We can do that later once we start digging in.

[svenevs]

That sounds good to me. The only snag I can see is the configuration file I needed above, and how to enable that for just one BUILD.bazel. Possibly I can provide the string and then the implementation generates it before running mypy? Or I check in the file and tell the bazel function to use it with a relative file path?

Not sure if local mypy configs are a good idea long term, or if there should just be one global drake one. Probably the latter, and likely we'll also want to install additional package-types from pip as more code tries to use mypy.

We may want to consider allowing a single BUILD.bazel to provide command line arguments to mypy. The example above I needed --explicit-package-bases for convoluted import reasons + the docker / macOS split that is slightly awkward and unlikely to come up elsewhere.

Edit: also, MYPYPATH is unlikely to be needed elsewhere. Just because of how that code has to be organized (image module and docker primarily).

[jwnimmer-tri]

Yes, nominally any config file(s) should be Drake-wide.

... for convoluted import reasons + the docker / macOS split that is slightly awkward and unlikely to come up elsewhere.

Those hijinks (sys.path manual hacking) are defective anyway, and I plan to rewrite that code sooner or later to avoid it.

[jwnimmer-tri]

The higher priority is isort and black. (In other words, help our contributors with code formatting and auto-fixing.) The mypy would be a second step.

Another fair candidate for the second phase would be flake8.

[jwnimmer-tri]

The code at https://github.com/RobotLocomotion/drake-blender/blob/main/tools/defs.bzl might be a useful starting point (not sure)?

This might end up blocked on #8392. If our various platforms don't have a sufficiently sync'd version of the linter available, we might cause problems for our developers if the rules fight each other. We might need to uniformly pin the linters using requirements.txt, which would mean we need rules_python to pull in the pip stuff for us.

[svenevs]

I haven't looked much, just stumbled across a possibly interesting alternative: https://docs.astral.sh/ruff/

[jwnimmer-tri]

I'm going to close #14234 as a duplicate of this. As mentioned upthread, another tool we can consider here is flake8.