-
Notifications
You must be signed in to change notification settings - Fork 226
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
Add typing / drop Python < 3.7 support #458
Conversation
- Run black -l 100 -t py37 - Run isort
This is mostly done, just need to cleanup ready for final PR. CC: @seanthegeek |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #458 +/- ##
==========================================
+ Coverage 58.42% 58.54% +0.11%
==========================================
Files 11 11
Lines 1347 1358 +11
==========================================
+ Hits 787 795 +8
- Misses 560 563 +3 ☔ View full report in Codecov by Sentry. |
Looks like this is mostly because there are overall more lines of code so even though it functionally has barely changed, the percentage of lines covered has actually dropped. I think we can ignore the coverage change in this instance. |
I'm not willing to drop Python 3.6 support for a while because RHEL 8 and its derivatives use Python 3.6. The end of life of most popular RHEL 8 derivative, Rocky Linux 8 isn't until May 31, 2029. That said, I'd like to include as much typing information is possible while still maintaining support for RHEL 8 and its derivatives. |
Thanks for reviewing this @seanthegeek :) Given that postponed annotations ( It might be worth formalising the supported versions and including / linking to them in the contributing docs. As an aside: I've decided to continue working on my forked repository to experiment with a new way of running the application. Given that it will not be python 3.6 compatible I don't think it will be possible to merge it (in addition to the many breaking changes). Assuming my experiment is successful I will end completely forking it (including changing the name and releasing it on PyPI). If you wish to discuss this, feel free to reach out to me directly. |
This PR adds Python 3.7+ compatible type annotations (dropping support for <3.7 at the same time).
fixes: #455
At a high level the following changes have been made:
black
andisort
formatterTest Plan
flake8
without errorspython3 tests.py
passes without errorsmypy
passes without errors.Open Questions
Q1: Currently I've left all instances of
OrderedDict
alone as I'm not sure if we're using other features of the class. If we're only using the fact that they keep insertion order we should convert them all todict
since as of 3.7 they are guaranteed to retain insertion order.That said, I think it would be better to move to using something like Dataclasses or Pydantic to properly type the report objects.