Drop python2 support #70
Conversation
| matrix: | ||
| python-version: [ | ||
| 2.7, | ||
| 3.5, |
There was a problem hiding this comment.
Python 3.5 was reached end-of-life last year.
There was a problem hiding this comment.
Let's make a separate PR to remove 3.5 and merge it faster. I see no reason not to.
There was a problem hiding this comment.
Ok, reverted and send the pr to remove python3.5.
#71
| On-hold until discussion #4 (comment) is resolved. |
yuokada force-pushed the pyupgrade3 branch 2 times, most recently from 71faab2 to 6fe5a7a Compare 
| keys = list(param.keys()) | ||
| values = [param[key] for key in keys] | ||
| return "MAP(%s, %s)" % ( | ||
| return "MAP({}, {})".format( |
There was a problem hiding this comment.
Just as a note, when we drop support for 3.5 in the subsequent PR as discussed this could be replaced with an f-string instead.
ebyhr left a comment
ebyhr left a comment There was a problem hiding this comment.
Looks good except for minor comment. Please update README too > "It supports Python 2.7..."
tox.ini Outdated
| @@ -1,5 +1,5 @@ | |||
| [tox] | |||
| envlist = py27,py35,py36,py37,py38,py39,pypy2 | |||
| envlist = py35,py36,py37,py38,py39,pypy2 | |||
There was a problem hiding this comment.
I think so. Removed.
ebyhr left a comment
ebyhr left a comment There was a problem hiding this comment.
Could you squash commits and update README?
#70 (review)
| @ebyhr Modified README.md. PTAL. |
hashhar left a comment
hashhar left a comment There was a problem hiding this comment.
Looks good overall % removal of pypy test covarage.
| @@ -1,5 +1,5 @@ | |||
| [tox] | |||
| envlist = py27,py35,py36,py37,py38,py39,pypy2 | |||
There was a problem hiding this comment.
Are we losing test coverage for pypy?
There was a problem hiding this comment.
Is it better to add pypy35 or something to test with pypy3X?
https://github.com/tox-dev/tox/blob/69af6cdf168df4e82d6ce0f3d05571bd100418f9/docs/example/basic.rst
There was a problem hiding this comment.
It would surely be good to do that in this PR itself if the tests are passing.
If CI fails on pypy3x let's create an issue for the follow-up.
There was a problem hiding this comment.
Looks like we aren't using tox in CI and instead directly run pytest https://github.com/trinodb/trino-python-client/blob/master/.github/workflows/ci.yml#L31.
Let's do this in a follow-up since this looks pre-existing anyway.
cc: @joshthoward
yuokada force-pushed the pyupgrade3 branch 2 times, most recently from da6cfa0 to b7ee8fa Compare 
| Hey all! Just wanted to comment that the current pypy release is 3.7 (also supported by tox) & 3.8 is around the corner; 3.5 is quite old (and doesn't seem to make much sense in the face of #77) |
| Merged, thanks for your work! @yuokada |
7 participants
Resolve #4