-
Notifications
You must be signed in to change notification settings - Fork 69
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
Drop Python 3.8 support #525
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #525 +/- ##
=======================================
Coverage 97.18% 97.18%
=======================================
Files 8 8
Lines 1064 1064
Branches 57 49 -8
=======================================
Hits 1034 1034
Misses 18 18
Partials 12 12 ☔ View full report in Codecov by Sentry. |
uses: actions/setup-python@v5 | ||
with: | ||
python-version: 3.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep to older versions for lint job, it's more likely to catch mypy errors that user's of older Python versions may encounter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
Ideally we should do mypy checks for all supported Pythons, it is not that hard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not necessarily against it, but from I've seen it doesn't really provide any benefit. I don't think we've ever encountered errors on a newer Python version than what we tested with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A newer version could raise a deprecation warning.
Honestly I don't expect any error from testing mypy against the oldest supported version as well; but if we can and want providing the possible strongest typing -- why don't do it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A newer version could raise a deprecation warning.
mypy wouldn't give a deprecation warning, that's a runtime thing. If it did, we wouldn't be able to do anything as we still need the code working on the older version.
Honestly I don't expect any error from testing mypy against the oldest supported version as well;
We've had a couple of occasions where this has happened. There's some slight oddities where mypy will produce an error (e.g. due to invalid syntax) when run on older versions of Python (as it's uses the Python parser), but won't produce those errors on newer versions.
but if we can and want providing the possible strongest typing -- why don't do it?
Just because it's unlikely to add anything, but it will add CI time and might add some complexity when some internal code produces different errors on different versions (making it difficult to pass on all versions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a minor thing from my perspective, but...
- Mypy supports deprecations: https://peps.python.org/pep-0702/
I guess in typeshed it will be implemented asif sys.version_info ...
, we can handle it as well. - Compilation issues don't belong to mypy entirely, if python 3.9 can compile then mypy for python 3.9 can process. Type checking for all python versions helps us to mitigate things like using API from newer python but covering it with the test with skip mark for older python versions. It is a very rare case, I agree, but still.
- CI time is the question. I suggest adding this mode for aiojobs only and see how it is going. The project is not updated frequently, we can experiment on it for free.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Mypy supports deprecations: https://peps.python.org/pep-0702/ I guess in typeshed it will be implemented as `if sys.version_info ...`, we can handle it as well.
I think that's still unsupported: https://htmlpreview.github.io/?https://github.com/python/typing/blob/main/conformance/results/results.html
2. Compilation issues don't belong to mypy entirely, if python 3.9 can compile then mypy for python 3.9 can process.
My point is more that we can have an issue with syntax that only work in 3.12+ and mypy doesn't detect the problem unless it's being run with an older version that doesn't support the syntax. I think that's roughly what happened before, when we saw issues.
Feel free to experiment though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, very cool table. I never saw anything like this.
Maybe we should switch to pyright if it is so good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think mypy is a solid foundation that most of our users use. It might be worth adding pyright as a second tool, but most of those features we won't use for some time as we support older versions still (e.g. deprecated was added in 3.13). mypy should have those features long before we actually use them.
No description provided.