Use pybind11 in _c_internal_utils module#26794
Merged
oscargus merged 2 commits intomatplotlib:mainfrom Nov 10, 2023
Merged
Conversation
13 tasks
QuLogic
force-pushed
the
internal-pybind11
branch
from
6270f03 to
0fca6f9
Compare
QuLogic
force-pushed
the
internal-pybind11
branch
from
0fca6f9 to
79c080b
Compare
Member
|
I think changing to a capsule is a good idea (maybe we want an API change note?). I lean to leaving the name. Even though it is cpp, Python still calls them "C-extensions" so being a bit imprecise and leaving the name as |
Member
Author
It is private at least, so no need? |
QuLogic
force-pushed
the
internal-pybind11
branch
from
79c080b to
bdcff91
Compare
This requires a minor bit of typecasting as in the `_tkagg.cpp` file. This is a separate commit from the pybind11 change to improve rename detection.
QuLogic
force-pushed
the
internal-pybind11
branch
from
bdcff91 to
a567006
Compare
oscargus
approved these changes
Oct 11, 2023
QuLogic
force-pushed
the
internal-pybind11
branch
from
a567006 to
f33f6cf
Compare
ianthomas23
approved these changes
Nov 9, 2023
Member
ianthomas23
left a comment
ianthomas23
left a comment
There was a problem hiding this comment.
I'd be inclined to remove the 4 uses of void in function argument lists now that these functions are C++ rather than C, but I don't feel strongly about this.
Member
|
Let's merge this and the voids can possibly be handled later. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR summary
I changed to a C++ file, in order to use pybind11, though I'm not sure if the name should change. I also made the
HWNDinto a capsule so we could check types a bit more stringently, though this is assuming no-one ever wants to callSetForegroundWindowwith a handle obtained elsewhere.This depends on the Meson PR, as it was much easier to iterate with its automatic re-compile in editable installs.
PR checklist