[v2] Modernize repo with latest python standards #2140
Conversation
While working on new ideas for v2, it'll be easier to focus on supporting a single python version, instead of all the complexities in supporting multiple versions of Python. We can then work out what versions we want to support based on when this is released.
This also moves all the dependencies specifications into the pyproject.toml file.
Still keeping `pylint -E` around because it catches issues that ruff does not catch yet. We can still remove the custom plugins for pylint though, not worth maintaining anymore.
Need to do this to use the venv created by uv.
|
Ah, so I had to remove the CDK test runs from the GitHub test run action. My original branch removed the CDK feature entirely but I wanted to keep this PR focused on repo modernization for now (I'll send a follow up PR to remove the actual code bits related to CDK). FWIW I'd still like to support CDK apps, but I have a different idea in mind for how to do it and I'd also like this to be in a separate package. |
pyproject.toml
Outdated
| "pylint", | ||
| "doc8<1.0.0", | ||
| "pydocstyle", | ||
| "flake8", |
There was a problem hiding this comment.
Do we know the specific class of things that pylint is catching but ruff is not?
Also do we still need flake8? I was under the impression that could all be handled by ruff as well.
There was a problem hiding this comment.
Removed flake8.
Do we know the specific class of things that pylint is catching but ruff is not?
It's been a while since I originally made this change, but IIRC ruff still can't do multi-file analysis, and there was a class of errors in that space that weren't detected by a typechecker (maybe they were untyped so they weren't processed by mypy?), not detected by ruff, but still flagged by pylint -E.
There was a problem hiding this comment.
Ok, no worries. May be good to create an issue/comment later to record any blockers to get us off pylint, but this is fine for now.
| @@ -17,39 +20,21 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ubuntu-latest, macos-latest] | |||
| python-version: [3.8, 3.9, '3.10', 3.11, 3.12] | |||
| python-version: [3.12] | |||
There was a problem hiding this comment.
Actually, last comment, I know we say we support 3.12+ and Lambda doesn't currently support 3.13. Would it hurt to test 3.13 now so things are ready to go as soon as Lambda releases?
There was a problem hiding this comment.
Nevermind, they already released 3.13. Would that be worth adding?
There was a problem hiding this comment.
I'll sync with @jonathan343 on this, but I had assumed we'd need #2137 to land before we can test on 3.13 (and pass). If we're still targeting the master branch for that PR, which I think makes sense, once it's merged we can rebase it into v2.
This modernizes the repo with the latest python standards for easier contribution
Python version support - 3.12+
My thinking here is that for v2, we'll start with only worry about python 3.12/3.13 for now. I want the focus to be on an updated set of features if we only support modern python, and leave the discussion of what versions of python to support as a separate discussion. This is because 1) we don't know when 2.0 will be finished and what the set of supported python vesrions will be with Lambda / botocore 2) In modern versions of python, the main differences in practice are minor typing enhancements and stdlib updates, very few language features I think we'd actually use that aren't available in 3.9/3.10+. The effort of adding older Python version support after the fact is much easier than the py3/py2 times so I think it will be minimal work.
In addition to python version I've also:
pyproject.tomlrufffor lintint. Note that ruff did not catch all the issues thatwe could previously catch so it's unfortunately not a drop in replacement.
As a result we still have a
pylint -Epass nwe need.cc @jonathan343 @nateprewitt @SamRemis