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

Migrate to Cython3 #813

Merged
merged 5 commits into from
Nov 4, 2024
Merged

Migrate to Cython3 #813

merged 5 commits into from
Nov 4, 2024

Conversation

ZipFile
Copy link
Contributor

@ZipFile ZipFile commented Aug 13, 2024

As title says. Major incompatibility with v0.29 was new mandatory dunder mangling, so solution was simply to rename them. Otherwise migration was pretty much straightforward.

Notes:

  • Cython 3 no longer recommends commiting C files. I already have some drafts, will submit PR after this one is merged.
  • So far it builds and runs on CPython 3.7-3.12 (with coverage).
  • On PyPy it won't build with DEPENDENCY_INJECTOR_DEBUG_MODE (probably Cython issue), without debug mode looks like it builds and runs fine (no coverage ofc), but there are few failing tests, need investigate more if this is related to version of PyPy itself (I was testing with 7.3.16).
  • Not exactly sure about segfaults mentioned in Roadmap 2024 #812, but majority of tests were failing because Cython 3 started mangling attributes that were previously unmangled.

@rmk135
Copy link
Member

rmk135 commented Aug 13, 2024

@ZipFile wow, thanks for the PR!

@rmk135 rmk135 self-assigned this Aug 13, 2024
@coveralls
Copy link

Coverage Status

coverage: 91.623%. remained the same
when pulling cb81e56 on ZipFile:cython3
into 2c998b8 on ets-labs:master.

@ZipFile
Copy link
Contributor Author

ZipFile commented Aug 13, 2024

Looks like tox was ignoring DEPENDENCY_INJECTOR_DEBUG_MODE env var set from GHA pipeline, coverage supposed to be around 94% with *.pyx included. Pushed fix + extra logging.

@oleksandr-kuzmenko
Copy link

@ZipFile thanks for the PR, It looks cool 👏

Hi @rmk135, would you mind taking a look at this PR when you have a chance? 🙏

Is there anything else needed to move it forward? I can try to help if needed

@rmk135 rmk135 changed the base branch from master to develop November 4, 2024 01:48
@rmk135 rmk135 merged commit 595daeb into ets-labs:develop Nov 4, 2024
@ZipFile ZipFile deleted the cython3 branch November 4, 2024 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants