Add Pre-Commit Python Linting

[hello-amal]

Description

This PR, in partial service of #43 , builds off of #42 to add Python linting to the pre-commit hooks.

Testing procedure

• Re-run all tests from Add Pre-Commit Hooks #42 with the following Stretch Configurations:
  • Stretch RE2 + no dex wrist w/ beta teleop kit
  • Stretch RE2 + dex wrist w/ beta teleop kit
  • Stretch RE2 + dex wrist w/ d405
  • Stretch 3 + current teleop kit + dex wrist

Before opening a pull request

From the top-level of this repository, run:

• pre-commit run --all-files

To merge

• Squash & Merge

[hello-binit]

What are your thoughts on flake8 vs Black for linting Python code?

[hello-amal]

They are complementary. black is a formatter, so it automatically formats the code (e.g., for readability and consistency). flake8 is a linter, so it points out potential style and syntax errors. There are some overlaps (like with line length), but generally both are useful. For example, a linter will point out things like a missing import, an unused import/variable, catching too broad of an exception, etc., which a formatter will not.

[hello-binit]

Gotcha, so this PR adds the linter and optional type checking (with Mypy), but not the formatter. Is that correct?

[hello-amal]

Yes. The previous PR (#42 ) added the formatter.

The reason I split it up this way is that the formatter is guaranteed to not change code functionality. Whereas when I implement linting fixes, it is possible to change code functionality. So I wanted them to be in two separate PRs, to be able to more easily test and compare the diff.

[hello-binit]

Great, that makes sense. Since you've confirmed that no functionality is broken from the changes in these large PRs, I've approved these two PRs (#42 and this one). Are you planning on merging them into mainline?

[hello-amal]

Yes I'll merge them. I'm just waiting on to test it on a Stretch 2 without a dex wrist, and will then merge them in.

[hello-binit]

Cool! I have one, so I can test this next week if you need it.

[hello-amal]

Merging without testing on the "Stretch RE2 + no dex wrist w/ beta teleop kit" configuration because:

1. I've double-checked the code, plus we've already tested with the beta teleop kit, so I'm fairly confident that it is unlikely to break that test case.
2. We haven't yet been able to procure that specific configuration.
3. Few users still use that configuration.