Skip to content

Conversation

@lysnikolaou
Copy link
Member

No description provided.

@lysnikolaou lysnikolaou added the documentation Improvements or additions to documentation label Jul 15, 2024
Copy link
Member

@rgommers rgommers left a 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 <[email protected]>
@lysnikolaou
Copy link
Member Author

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.

Copy link
Collaborator

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.

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Almost there!

@rgommers rgommers merged commit c12572b into Quansight-Labs:main Jul 16, 2024
@rgommers
Copy link
Member

Merged, thanks @lysnikolaou and @ngoldbaum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants