Skip to content

Fix cython3 #35675

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

Closed
wants to merge 3 commits into from
Closed

Fix cython3 #35675

wants to merge 3 commits into from

Conversation

tacaswell
Copy link
Contributor

I'm not sure if this actually works, but it gets cython to compile with py310.

@jbrockmendel
Copy link
Member

Should this take over from #34997?

@tacaswell
Copy link
Contributor Author

Ah, I missed #34997. I probably should pick off the second commit into its own PR and then sort out if my attempt here is the right path or the one in the other PR.

That said, my guess is that this code is performance critical and was written the way it was as an optimization and I really don't have more than a passing understanding of the block manager so I am not the right person (and don't really have the bandwidth right now to become the right person) to take this on...

@@ -144,7 +144,7 @@ cpdef inline Py_ssize_t word_len(object val):
Py_ssize_t l = 0

if isinstance(val, str):
l = PyUnicode_GET_SIZE(val)
l = PyUnicode_GET_LENGTH(val)
Copy link
Member

Choose a reason for hiding this comment

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

this can be done independently of the rest of the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, do you want to split it out?

@tacaswell
Copy link
Contributor Author

Sorry for both the low-quality PR and dropping the ball on moving this forward. I had this sitting on my machine for a while, thought it was better to get it out in the public at all and that I would have more bandwidth to follow up :(.

@WillAyd WillAyd closed this Sep 9, 2020
@WillAyd
Copy link
Member

WillAyd commented Sep 9, 2020

All good @tacaswell ! I think this is getting covered in some other PRs so closing this one to clear the queue, but effort much appreciated in any case

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.

Update setting data pointers for Cython 3
3 participants