Skip to content

Resolve C++ nullptr issues and move away from deprecated C API usages in relation to #31424 - #32065

Open
clin1234 wants to merge 7 commits into
matplotlib:mainfrom
clin1234:freethreaded_fixes
Open

Resolve C++ nullptr issues and move away from deprecated C API usages in relation to #31424#32065
clin1234 wants to merge 7 commits into
matplotlib:mainfrom
clin1234:freethreaded_fixes

Conversation

@clin1234

@clin1234 clin1234 commented Jul 18, 2026

Copy link
Copy Markdown

PR summary

Attempt to fix (most) trivial C++ bugs found in #31424 during a sprint in SciPy 2026.

@github-actions

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@clin1234
clin1234 force-pushed the freethreaded_fixes branch from 6e1ed1d to 03e3c94 Compare July 18, 2026 19:40
@clin1234
clin1234 marked this pull request as ready for review July 18, 2026 20:38
Comment thread src/ft2font_wrapper.cpp Outdated
Comment thread src/ft2font_wrapper.cpp Outdated
@github-actions github-actions Bot added CI: Run cibuildwheel Run wheel building tests on a PR backend: agg topic: images topic: tri and removed CI: Run cibuildwheel Run wheel building tests on a PR labels Jul 19, 2026
@clin1234
clin1234 force-pushed the freethreaded_fixes branch 4 times, most recently from a44a307 to 2ed73b3 Compare July 19, 2026 19:08
@clin1234
clin1234 force-pushed the freethreaded_fixes branch 2 times, most recently from 7095fc2 to 6b1521a Compare July 19, 2026 21:11
@QuLogic

QuLogic commented Jul 20, 2026

Copy link
Copy Markdown
Member

Much of this conflicts with #30291 and ft2font-shared_ptr; it wasn't merged for 3.11 because pybind11 v3 was deemed too new at the time.

@clin1234

Copy link
Copy Markdown
Author

Much of this conflicts with #30291 and ft2font-shared_ptr; it wasn't merged for 3.11 because pybind11 v3 was deemed too new at the time.

Would you be willing to rebase your tf2font-shared_ptr branch onto my PR branch? Afaik, you haven't made a PR from that shared_ptr branch yet

Comment thread src/ft2font_wrapper.cpp Outdated
Comment thread src/ft2font_wrapper.cpp
@tacaswell

Copy link
Copy Markdown
Member

Given that this is doing a bunch of different things it seemed easier to review it commit-by-commit, but a number of the commits either stack or are later reverted. @clin1234 Is it possible to squash this down so the reverted commits are fully dropped and the commits that stack (like adding missing ;) are squashed it to one commit?

@clin1234

Copy link
Copy Markdown
Author

Given that this is doing a bunch of different things it seemed easier to review it commit-by-commit, but a number of the commits either stack or are later reverted. @clin1234 Is it possible to squash this down so the reverted commits are fully dropped and the commits that stack (like adding missing ;) are squashed it to one commit?

Would you prefer that I squash all commits within this PR to a single one?

@tacaswell

Copy link
Copy Markdown
Member

Would you prefer that I squash all commits within this PR to a single one?

Can you aim for 3-4 commits that each do one self-contained thing? That will make it much easier to review.

@clin1234
clin1234 force-pushed the freethreaded_fixes branch from d446b0c to 49482e9 Compare July 25, 2026 14:38
@clin1234
clin1234 force-pushed the freethreaded_fixes branch 2 times, most recently from 506d46e to 09ff143 Compare July 25, 2026 15:04
clin1234 added 3 commits July 25, 2026 12:26
In particular:
* ranged-for in `ft_glyph_warn`
* PyErr_GetRaisedException and PyErr_SetRaisedException replaces PyErr_Fetch and PyErr_Restore
* `PyFT2Font_init` returns a std::unique_ptr<PyFT2Font>
* Make face_index within PyFT2Font_init FT_ULong
In particular:
* Return descriptive error if FT2Image can't be constructed
* Fully initialize FT_Outline_Funcs
* Use `THROW_FT_ERROR` in place of bare std::runtime_error
@clin1234
clin1234 force-pushed the freethreaded_fixes branch from 09ff143 to 01dcacc Compare July 25, 2026 16:27
@clin1234

Copy link
Copy Markdown
Author

@tacaswell Done

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.

4 participants