Update pre-commit hooks, dropping support for development on 3.6-3.7 #371
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Vermin 1.6.0 was recently released, which contains a fix for netromdk/vermin#230. This was a rather odd corner case, in which Vermin would fail when code was compatible with 2.x and 3.x, yet the command line only required compatibility with 3.x. Since pre-commit runs against only files that changed, this was possible, for example with small test files. I'm not sure if I encountered it in drgn (I know I did with drgn-tools), but it's good to have the fix. I used pre-commit autoupdate, so we have the latest version of each hook now. In particular, this commit surpasses mypy 0.971, which is the last version to support Python 3.6 at runtime. Mypy still supports targeting Python 3.6[1]. [1]: https://mypy-lang.blogspot.com/2022/07/mypy-0971-released.html Two new errors were surfaced with the new hooks: 1. Mypy complained about the pattern "os.exit(exception)". I've replaced these so they explicitly use str: "os.exit(str(exception))". drgn/cli.py:283: error: Argument 1 to "exit" has incompatible type "OSError"; expected "str | int | None" [arg-type] 2. Vermin complained that "int.is_integer()" was introduced 3.12. I surmised that it was unable to infer that its usage was guaranteed to be against a float, and the reason was due to the division operation against an integer. I changed the integer literal to a float literal, which resolved the issue. !2, 3.12 drgn/helpers/common/format.py 'int.is_integer' member requires !2, 3.12 Signed-off-by: Stephen Brennan <[email protected]>
Since updating pre-commit hooks, Python 3.6 and 3.7 will no longer pass pre-commit runs. Skip installing and running pre-commit/mypy on the unsupported versions of Python. To do this, we (ab)use Bash's regex to detect supported versions, and set an environment variable when pre-commit should be used. Then, we (ab)use parameter expansion syntax and Github Actions if-conditionals to decide whether to skip/run. Signed-off-by: Stephen Brennan <[email protected]>
Since updating pre-commit and the hooks, development on EOL Python versions is no longer supported, and will fail. Update the contributing guide to explain that Python 3.6 is supported for build, runtime, and tests, but development workflow scripts require a supported Python version. Repeat this warning in the pre-commit section, where it is most relevant. Signed-off-by: Stephen Brennan <[email protected]>
|
This seems like a good compromise. For #364, I'd like to use some typing features that are only available in mypy versions that no longer support 3.6 and 3.7. This would break drgn library users that are type checking on 3.6 or 3.7. If there even are any, I don't feel too bad about it. What do you think? |
|
I obviously can't speak for all users, but I don't know of any use of type checkers on the Python 3.6/3.7 code. The docs we have for setting up development environments specify 3.8+ for the development python version. So nobody at Oracle is running mypy on those versions. As far as I'm concerned if this helps your type annotation work, then that's even better. |
|
Merged, thanks! This will make the annotations for #364 SO much easier. I'm catching up after Thanksgiving and a couple of sick days, but I'll have an update for that soon. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reflecting a bit of our discussion at Plumbers, I think this won't be too controversial. I've gone ahead and updated the pre-commit hooks to their latest versions, and handled any breakages there (see commit message for details). Of course, this blitzes past several tool versions which drop support for Python 3.6. So I've updated the CI to skip pre-commit/mypy on 3.6 / 3.7, and I've updated the contributing guide to explicitly state a policy on Python versions: 3.6+ is supported for building, running, & testing, but development tooling requires a non-EOL Python version.
Let me know your feelings on this, I think it will help alleviate some, but not all, of our woes with Python 3.6 support.