-
-
Notifications
You must be signed in to change notification settings - Fork 341
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
Make trio run on Python 3.13 (probably) #2918
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2918 +/- ##
=======================================
Coverage 99.64% 99.64%
=======================================
Files 116 116
Lines 17477 17477
Branches 3133 3133
=======================================
Hits 17415 17415
Misses 43 43
Partials 19 19
|
src/trio/_path.py
Outdated
@@ -199,6 +199,7 @@ def generate_iter(cls, attrs: dict[str, object]) -> None: | |||
setattr(cls, attr_name, wrapper) | |||
|
|||
|
|||
# TODO: `pathmod` points to `os.path`... is that what we want? |
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 left this because ATM trio.Path
has a pathmod
pointing to a synchronous path module. Which is weird but also we don't have an equivalent module IIRC.
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 might get changed, see python/cpython#113893.
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.
python/cpython#113893 was merged
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.
Looks like the TODO is irrelevant because AFAICT the pathmod
or flavour
or flavor
or whatever it will be only does operations like joining paths. Seemingly nothing that interacts with FS?
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.
Yea, as far as I remember looking at it a bit ago I think they've just make an abstract class for the path interface, at least in that particular PR
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 seems like the right approach for pathmod
, but we might want to consider expanding trio.Path
a bit in anticipation of 3.13. Since pathlib
paths can now be subclassed (to allow say paths inside a zip, or on a remote server), we might want to make it possible to wrap subclasses of path using this.
FYI, this patch let me fix urllib3's test with CPython 3.13 in urllib3/urllib3#3156. Thanks! |
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.
These changes look good to me!
I'm sorry, this has slipped my radar. I can run the tests with Python 3.13 now. (8 of them fail, but that's another tale.) |
I'm pretty sure I tested this 3 weeks ago locally. Unfortunately it's not possible to run 3.13 in CI ATM because there's no compatible
cryptography
release.This fixes #2903. (hopefully)
cc @befeleme can you make sure this is fine?