Skip to content

Cython 3 fixes #94

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Cython 3 fixes #94

wants to merge 2 commits into from

Conversation

SamHames
Copy link
Contributor

This fixes two things related to Cython 3 compatibility as raised in #93:

  1. Enables the legacy behaviour for noexcept clauses. I don't know Cython well enough to fix the underlying issue with the new semantics, but it seems to all focus on the binary_op and binary_iop methods.
  2. Changes __check_compatibility to _check_compatibility to avoid the name being mangled in subclasses.

These should both work fine with earlier Cython's as far as I understand.

SamHames added 2 commits July 27, 2023 08:40
@SamHames
Enable legacy_implicit_noexcept to work with Cython 3.0.0
7bc12aa 
Cython 3 changes the semantics of exception handling, so all
non-extern functions propagate exceptions be default. This is
documented here: https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html?highlight=noexcept#exception-values-and-noexcept

This change enables the legacy behaviour so builds can work
on Cython 3.
@SamHames
Fix Cython 3's changed handling of __methods
765c018 
Cython 3 now mangles classnames starting with __, just like in
regular Python - this means that __check_compatability was not
visible to the BitMap class.
@Ezibenroc
Copy link
Owner

Hello and thanks a lot for taking a look at this issue!
It seems the build is failing on all Python versions for MacOS. I found this related issue that is also quite recent: cython/cython#5568
I don't think that it comes from your PR though. Apparently, Cython 3.0.0 was released two weeks ago, and no other PR was done in this time window.

So, in the short term, I think it is best to just pin Cython version: #95
But as soon as this MacOS build is fixed (either on their side or on our side), I will gladly merge this and remove the constraint on Cython version.

This was referenced Jul 30, 2023
Merged
Closed
Open
@benjamb
Copy link

benjamb commented Jul 30, 2023

It doesn't have to be done as part of this PR, but it would be worthwhile fixing this properly, rather then relying on legacy_implicit_noexcept.

On an unrelated note: @Ezibenroc is it worth dropping 3.7 from the CI? Given it is now EOL.

@Ezibenroc
Copy link
Owner

It doesn't have to be done as part of this PR, but it would be worthwhile fixing this properly, rather then relying on legacy_implicit_noexcept.

Yes, fully agree.

On an unrelated note: @Ezibenroc is it worth dropping 3.7 from the CI? Given it is now EOL.

My process until now was to keep supporting the versions until something break, and only then remove them. Some companies and universities are stuck in old Python versions (even older than 3.7 sometimes), so upgrading too fast can be a pain.
But I don't want to add extra-work either, so as soon as 3.7 fails in some way, it will be removed yes 👍

@SamHames
Copy link
Contributor Author

SamHames commented Sep 3, 2023

Closing this as #97 does it properly.

@SamHames SamHames closed this Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SamHames @Ezibenroc @benjamb