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

Use pybind11 in _c_internal_utils module #26794

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Sep 16, 2023

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 HWND into a capsule so we could check types a bit more stringently, though this is assuming no-one ever wants to call SetForegroundWindow with 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

@tacaswell
Copy link
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 _c_internal_utils seems fine.

@QuLogic
Copy link
Member Author

QuLogic commented Oct 5, 2023

I think changing to a capsule is a good idea (maybe we want an API change note?).

It is private at least, so no need?

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.
Copy link
Member

@ianthomas23 ianthomas23 left a comment

Choose a reason for hiding this comment

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

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.

@oscargus
Copy link
Contributor

Let's merge this and the voids can possibly be handled later.

@oscargus oscargus merged commit c0e9ebc into matplotlib:main Nov 10, 2023
41 checks passed
@QuLogic QuLogic deleted the internal-pybind11 branch November 14, 2023 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants