-
Notifications
You must be signed in to change notification settings - Fork 24
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
Document Cython's freethreading_compatible directive #35
Conversation
01b5da4
to
15740be
Compare
15740be
to
2f787b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @lysnikolaou! A few suggested tweaks
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Thanks for the review @rgommers! Feedback addressed. |
(`# cython: freethreading_compatible=True`) in `.pyx` files, or globally by adding | ||
`-Xfreethreading_compatible=True` to the Cython arguments via the project's | ||
build system. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's some example code to do this via build configuration with setuptools and meson:
compiler_directives = {'freethreading_compatible': True}
setup(
ext_modules=cythonize(
extensions,
compiler_directives=compiler_directives)
)
cython_args = []
if cy.version().version_compare('>=3.1.0')
cython_args += ['-Xfreethreading_compatible=True']
endif
Meson compiler versions don't include alpha or beta qualifiers, so no need to check that for cython.
For the setuptools one, you could note that you can use e.g. packaging.version
to check for a minimum Cython version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of including example build config code - that'll definitely be helpful for users.
The Meson code should include an extension_module
call as well I think, something like:
py.extension_module('modulename'
'source.pyx',
cython_args: cython_args,
...
)
Even nicer if we had a CMake/scikit-build-core version of it as well. Maybe @henryiii could suggest an idiomatic snippet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks both for the suggestions! I added setuptools and Meson examples, but I don't know enough about CMake/scikit-build-core to do that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re scikit-build-core, I think this is a good example: https://github.com/scikit-build/scikit-build-sample-projects/tree/main/projects/hello-free-threading (or the hello-cython
next to it). However, it doesn't have the code yet to mark extension modules as compatible and I'm not sure what the idiomatic way to do it is, so let's do this as a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be better once we finish scikit-build/cython-cmake#19. Will try to do that this week and then update here.
4fd1b2c
to
a94ec7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Merged, thanks @lysnikolaou and @ngoldbaum. |
No description provided.