-
-
Notifications
You must be signed in to change notification settings - Fork 79
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 official support for current Python versions #353
Conversation
- remove support for Python 2.7 - adapt tests and CI accordingly - minor documentation updates
pull_request: | ||
branches: [ master ] |
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.
This has restricted the CI run to PRs from the master branch itself, which is usually not what is wanted.
@@ -1,5 +1,5 @@ | |||
# Testing and deployment packages. | |||
coverage==4.4.2 |
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.
Ideally, all this should go into a pyproject.yaml
, but I wanted to keep the changes small.
self.cached_total += 1 | ||
return self.cached_total | ||
|
||
return Check | ||
|
||
|
||
class TestCachedProperty(unittest.TestCase): | ||
class TestCachedProperty(unittest.IsolatedAsyncioTestCase): |
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.
unitttest.IsolatedAsyncioTestCase
is available since Python 3.8, so the tests won't run with Python 3.7 (which is EOL for some time now anyway), but the functionality will still work in 3.7, just not be "officially" supported.
""" | ||
Assert that both `add_control` and 'control_total` equal `expected` | ||
""" | ||
value = yield from check.add_control() # noqa | ||
self.assertEqual(value, expected) |
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 exchanged these as it is confusing if in the error output the actual and expected values are swapped - and I got quite a few failures on the first try...
@pydanny - I made this PR as the package is a dependency of another package we are using, and it looked unmaintained (no check-ins for 4 years, all CI tests fail). |
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.
@mrbean-bremen, thanks so much for this incredibly well-done and focused pull request. 😄
I'm merging it in and will be pushing a new release to PyPI as soon as I get part figured out for this particular project.
@mrbean-bremen, I just pushed up release 2.0! Thanks so much! https://pypi.org/project/cached-property/ |
Thanks for quick follow-up and the kind words! |
This shall not change any functionality, just got rid of Python 2.7 and outdated Python 3 version support, and adapted the CI to test all supported versions.
Note that I removed two tests that tested threading behavior with the version that is not thread-save, as the outcome of the test was unreliable due to race conditions. As this should not be a use case, I thought them unnecessary.