Skip to content

Update new_closure_bound closure signature#3883

Merged
davidhewitt merged 3 commits intoPyO3:mainfrom
LilyFirefly:pycfunction-new-closure
Feb 23, 2024
Merged

Update new_closure_bound closure signature#3883
davidhewitt merged 3 commits intoPyO3:mainfrom
LilyFirefly:pycfunction-new-closure

Conversation

@LilyFirefly
Copy link
Contributor

Following on from #3877, I noticed that PyCFunction::new_closure_bound has a bound that still uses the GIL ref api for the closure. This updates that to use the Bound api instead.

@LilyFirefly LilyFirefly added the CI-skip-changelog Skip checking changelog entry label Feb 22, 2024
@LilyFirefly LilyFirefly force-pushed the pycfunction-new-closure branch 2 times, most recently from 6ab1624 to fd380b1 Compare February 22, 2024 21:33
@LilyFirefly LilyFirefly marked this pull request as ready for review February 22, 2024 21:33
Copy link
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks for picking this one up! I have a suggestion, which is sort of for my own learning too 😂

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks both! One final thought from me...

LilyFirefly and others added 3 commits February 23, 2024 09:50
Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
Co-authored-by: David Hewitt <mail@davidhewitt.dev>
@LilyFirefly LilyFirefly force-pushed the pycfunction-new-closure branch from 191ce6e to c7b1e45 Compare February 23, 2024 10:45
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks both, this looks correct and elegantly implemented!

@davidhewitt davidhewitt added this pull request to the merge queue Feb 23, 2024
Merged via the queue into PyO3:main with commit e145ae8 Feb 23, 2024
@LilyFirefly LilyFirefly deleted the pycfunction-new-closure branch February 23, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants