Update Python support to 3.10+ - #198
Conversation
|
Could you please send some of the clean up work as separate PRs? For example, I see Python 2 removals in the "Update Python support to 3.10+" commit. Also, PR checks have not run on this PR. I'm not sure if a change here broken them or if there's an existing issue. |
Removes nose as test runner, adds pytest instead.
e4b25f2 to
1916ad0
Compare
str.format() calls replaced with f-strings.
socket.timeout is an alias of TimeoutError since Python 3.10.
In Python 2, IOError/OSError could be raised as a tuple (errno, message).
1916ad0 to
04c90bb
Compare
|
Hi @timgraham 👋 I think I've extracted out the modernizations from that commit. Though I disagree with you about splitting it up into multiple PRs. When the maintainer comes around, they can decide if they want to split things up or merge them all in one go. Hard to know what they'll want right now. I'm also happy to have someone else cherry-pick the commits or redo things. I was just trying to help things along. As for why the tests aren't running, it's because the action is triggered on |
|
I have merge permissions on this repo. I don't see why tests shouldn't run on PRs, so I have proposed it in #199. |
|
I merged a couple of the commits. Some commits still mix concerns (e.g. "Update Python support to 3.10+" changes the test runner, addings Python 3.14, removes usage of get_module_constant (reason unclear), makes unrelated changes to tox.ini). |
|
@timgraham Understood. I'm not going to have time to implement those changes. Happy to have you or someone else make those changes here or in a new PR. |
| version = re.search( | ||
| r'^__version__ = ["\']([^"\']+)["\']', | ||
| open("memcache.py", encoding="utf-8").read(), | ||
| re.M, | ||
| ).group(1) |
There was a problem hiding this comment.
What's the reasoning for the change? Because get_module_constant() is an internal API?
There was a problem hiding this comment.
The reasoning is that the LLM was probably overzealous in revising the code. Let me revert this one too. Thanks for catching it.
I don't believe we need to be worried about get_module_constant not being defined for Java backed pythons.
| [tox] | ||
| minversion = 1.6 | ||
| envlist = py{36,37,38,39,310,311,312},pypy,pep8 | ||
| envlist = py{310,311,312,313,314},pep8 |
There was a problem hiding this comment.
That looks like an oversight.
There was a problem hiding this comment.
@timgraham I tracked it down. It wasn't running in CI so it didn't make sense to run locally.
|
Thanks, I've merged most everything. |
2 participants
Sorry for the duplicate PR. I dropped the other change around setting a specific encoding as it seemed silly on re-review.