Skip to content
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 Python 3.13 Compatible #79

Merged
merged 12 commits into from
Jul 8, 2024
Merged

Make Python 3.13 Compatible #79

merged 12 commits into from
Jul 8, 2024

Conversation

FlickerSoul
Copy link
Contributor

This is currently a draft PR. See #72 (comment).

@FlickerSoul FlickerSoul marked this pull request as draft July 4, 2024 05:36
@edgarrmondragon
Copy link

I was working on

but if we can make the jump to pyo3 0.22 in this PR, I think that's preferable :)

@edgarrmondragon
Copy link

You need to add 3.13 to

python-version: |
3.8
3.9
3.10
3.11
3.12
pypy3.10

@FlickerSoul
Copy link
Contributor Author

FlickerSoul commented Jul 4, 2024

@edgarrmondragon Thank you so much for the tips! I'll take a look at your PR tomorrow and can merge anything that can be added.

I'm not quite confident in my implementation. If you have time, it would be great to hear your ideas. Thanks!

@edgarrmondragon
Copy link

@edgarrmondragon Thank you so much for the tips! I'll take a look at your PR tomorrow and can merge anything that can be added.

I'm not quite confident in my implementation. If you have time, it would be great to hear your ideas. Thanks!

Changes look good to me, but I'm also not super familiar with pyo3' API (old or new) 😅. Maybe @davidhewitt can take a quick look in case something's majorly fumbled here.

Copy link

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Broadly this looks correct to me 👍

src/lib.rs Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@FlickerSoul
Copy link
Contributor Author

@Julian Hi Julian, this PR should be ready for review. It bumps PyO3 to 0.22.1 and passes tests in Python 3.13 with GIL. However, it still breaks under the free-threaded Python. I'm happy to wait for updates from PyO3 and integrate so that rpds works on both variants of 3.13. What do you think?

@davidhewitt
Copy link

PyO3 will likely be a release or two before we support free-threaded Python properly, though it's very high priority on my TODO list.

I think probably we should be adjusting PyO3 to fail builds with a helpful error on free-threaded builds for the moment (maybe will release as 0.22.2). So I would advise not to wait for free-threaded support from us :)

@FlickerSoul
Copy link
Contributor Author

@davidhewitt Thank you so much for your insight! I can submit a PR to allow PyO3 fail builds on free-threaded Python if that's helpful. :)

@Julian
Copy link
Member

Julian commented Jul 8, 2024

Thank you all for your help here, it's really appreciated! I'm going to merge I think. Thanks again!

@Julian Julian marked this pull request as ready for review July 8, 2024 13:08
@Julian Julian merged commit 6c32fc2 into crate-py:main Jul 8, 2024
34 checks passed
@Julian Julian mentioned this pull request Jul 8, 2024
@FlickerSoul FlickerSoul deleted the dev branch July 8, 2024 13:20
@davidhewitt
Copy link

@davidhewitt Thank you so much for your insight! I can submit a PR to allow PyO3 fail builds on free-threaded Python if that's helpful. :)

@FlickerSoul a PR would be greatly appreciated. You'd need to modify pyo3-build-config to detect the freethreaded build and then emit an error similar to how we already check for supported Python versions.

@EwoutH
Copy link

EwoutH commented Jul 8, 2024

6 minutes ago I added 3.13 to my CI setup and rpds was failing. Looked up the issues, saw 3 minutes ago rpds v0.19.0 was released, re-ran and it passed.

Awesome timing, and thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants