Drop Python 3.7 #8572
Drop Python 3.7 #8572
Conversation
|
Follows from this community discussion: dask/community#213 |
|
What's the status on this @jrbourbeau? |
|
Thanks for the ping @jcrist. I just brought this PR back to life (there were several merge conflicts from some recent |
|
Excellent, thanks @jrbourbeau. With the CI errors on 3.7 windows, it'd be good to get this in soon (rather than pushing up an eventually unneeded CI fix). |
jcrist
left a comment
jcrist
left a comment
There was a problem hiding this comment.
I haven't done a full grep through to see if you've found all the 3.7 compatibility logic (worst case we delete more lines later), but the changes here look good to me.
|
Thanks @jcrist! I totally share the desire to get this in sooner rather than later. The only thing we should do is make sure this and dask/distributed#5683 go in at the same time (otherwise Python 3.7 CI builds will start failing) |
|
Sorry missed the ping earlier. Will try to look tonight or tomorrow. Thanks for working on these James 🙂 |
jakirkham
left a comment
jakirkham
left a comment
There was a problem hiding this comment.
Thanks James! 😄
Generally looks good. Had one question below 🙂
| return acc | ||
|
|
||
|
|
||
| _PY_VERSION = parse_version(".".join(map(str, sys.version_info[:3]))) |
There was a problem hiding this comment.
This doesn't appear to be used anywhere (including Dask generally). Should we just drop this? Maybe this whole module?
There was a problem hiding this comment.
Good point. In general, I tend to agree that if something isn't used we should just drop it. That said, _PY_VERSION has been used in the past and, while it's not needed at the moment, I anticipate it will be useful again in the future (e.g. xfailing a test on a specific Python version). So in this particular case I'm inclined to keep it around, but I don't think it's a huge deal either way (let me know if you'd like it removed)
|
Thanks @jakirkham @jcrist -- planning to merge this and dask/distributed#5683 later today if no further comment |
|
Thanks James for the PR and everyone for the reviews! 😄 |
|
Woo! |
Just noticed [these]() errors in the CI for python 3.7. It seems related to dask, and I noticed that dask does not support 3.7 since over a [year](dask/dask#8572). So I am removing 3.7 support here.
* Drop Python 3.7 Just noticed [these]() errors in the CI for python 3.7. It seems related to dask, and I noticed that dask does not support 3.7 since over a [year](dask/dask#8572). So I am removing 3.7 support here. * Update ci.yaml * Update whats-new.rst
4 participants
This PR removes support for Python 3.7. Ideally this would be merged around the same time as #8566
EDIT: Closes #2568