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

Freethreading to nogil #6210

Closed
wants to merge 1 commit into from
Closed

Conversation

da-woods
Copy link
Contributor

For naming consistency.

#6199 (comment)

For naming consistency
def put_acquire_freethreading_lock(self):
def put_acquire_nogil_lock(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, seeing this now makes it very ambiguous with Cython's nogil features. That's even worse because it really gives the wrong context. We use "nogil" in Cython's sources all over the place.

A name like "…cpython_nogil…" doesn't seem clear either. Maybe "freethreading" in one word isn't bad after all? _CPYTHON_NOGIL for the target macro and *_freethreading_* in our code base?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree (now you point that out).

I wonder if we should use "freethreading" everywhere (since it's what Python has chosen to call the build). But I'd also be happy to leave the target macro unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Let's change it everywhere, also in the target macro name. We're freshly introducing it for 3.1, so let's use something really unambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK - that's at #6213

@da-woods da-woods closed this May 23, 2024
@da-woods da-woods deleted the freethreading_rename branch May 23, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants