-
Notifications
You must be signed in to change notification settings - Fork 42
Fix python311 compatibility #69
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
Conversation
You'll need "3.11-dev" until 3.11 is released, and currently installing |
Thanks! Do you mind giving trying to set up the CI? I think it would be good to make sure, we don't break anything else wrt Python 3.11. |
I'll set it up later tonight 👍 |
91813b9
to
470a433
Compare
I added tests for mkdir's exist_ok behavior, which required a few additional changes to the tests for cloud filesystems. Regarding the py3.11 tests: I will wait for aio-libs/aiohttp#6600 to provide sources that build under 3.11 which should hopefully be soon, now that rc1 is out. |
89b1271
to
bf1ea06
Compare
mkdir was the one method broken by moving away from _accessors in Python3.11
bf1ea06
to
a1dd8fd
Compare
a1dd8fd
to
4366b97
Compare
Hey everyone, This PR is now ready for review 👍 Cheers, |
if create_parents: | ||
return self._fs.makedirs(pth, **kwargs) | ||
else: | ||
if not kwargs.get("exist_ok", False) and self._fs.exists(pth): |
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.
why not put exist_ok
explicitly in the function signature ?
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'm using the function signature defined in the base accessor cls previously defined in #59
I don't think it's worth cleaning this up a lot, since (1) it's a private interface and (2) all of this has to be refactored again once the remaining pathlib changes land in python=3.12.
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.
LGTM! Thanks for this PR!
Hello everyone,
this fixes all tests under Python 3.11. And it closes #67.
It basically only required to provide a working
mkdir
method for the testsuite to pass.In Python 3.11's pathlib the
_accessor
classes have been removed and so UPath's implementation deviates now a little bit from the new pathlib implementation. It's not really problematic for now, if we explicitly disable all that is not working (I raiseNotImplementedError
in many methods that now rely directly onos.path
functions). If needed, some of these disabled methods could probably be implemented in an fsspec compatible way.Once python/cpython#31691 and later python/cpython#31085 are merged into python, I believe this should be revisited.
Cheers,
Andreas 😃