Skip to content

Cython3 compatibility #97

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

Merged
merged 6 commits into from
Sep 3, 2023
Merged

Conversation

SamHames
Copy link
Contributor

@SamHames SamHames commented Sep 3, 2023

This makes the necessary changes for Cython 3 compatibility, while still working with Cython <3. This PR removes the need to use the legacy_implicit_noexcept directive.

I've also changed the way wheels are built for the test_wheels tox environment, now using python -m build to create them in an isolated environment. This is because I was running into the same test failures on MacOSX as my last PR - I'm not 100% sure if that's even a Cython only problem, or a combination of Cython + Tox + installation things. I'd investigate further into why this works, but I don't have access to an OSX environment. I'd expect most people will be using prebuilt wheels anyway, and the standard local source install test cases work without changes anyway.

- Change __check_compatibility to _check_compatability, as Cython3
  mangles names starting with __ in inherited classes like Python
- Explicitly declare noexcept in relevant type signatures so that
  we don't need the legacy_implicit_noexcept directive.
- This should still be compatible with Cython <3
This should be in a separate environment - this is mostly to
triangulate the CI related failures on macosx. If it does work
this is simpler anyway!
@SamHames SamHames mentioned this pull request Sep 3, 2023
@Ezibenroc
Copy link
Owner

Awesome, thanks a lot!

@Ezibenroc Ezibenroc merged commit 3b07395 into Ezibenroc:master Sep 3, 2023
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.

2 participants