X Tutup
Skip to content

agg: Replace facepair_t with std::optional#30196

Merged
anntzer merged 1 commit intomatplotlib:mainfrom
QuLogic:optional-face
Jul 1, 2025
Merged

agg: Replace facepair_t with std::optional#30196
anntzer merged 1 commit intomatplotlib:mainfrom
QuLogic:optional-face

Conversation

@QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 20, 2025

PR summary

This type seems to cover the intent more clearly than std::pair<bool, ...>.

PR checklist

@QuLogic QuLogic added this to the v3.11.0 milestone Jun 20, 2025
@QuLogic QuLogic added the CI: Run cibuildwheel Run wheel building tests on a PR label Jun 20, 2025
@github-actions github-actions bot added backend: agg and removed CI: Run cibuildwheel Run wheel building tests on a PR labels Jun 20, 2025
@QuLogic
Copy link
Member Author

QuLogic commented Jun 20, 2025

Our macOS deployment target is too old to like std::optional::value(), so I've changed it to std::optional::operator*.

@QuLogic QuLogic added the CI: Run cibuildwheel Run wheel building tests on a PR label Jun 20, 2025
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I would decide on one of the APIs

  • use pointer-like syntax if (face), *face
  • use explicit syntax if (face.has_value()), face.value()

but not mix them.

This type seems to cover the intent more clearly than `std::pair<bool,
...>`.
@github-actions github-actions bot removed the CI: Run cibuildwheel Run wheel building tests on a PR label Jun 21, 2025
@QuLogic
Copy link
Member Author

QuLogic commented Jun 21, 2025

Well, we can't do face.value(), so it'll have to be the former.

@anntzer anntzer merged commit f6ae9e7 into matplotlib:main Jul 1, 2025
45 checks passed
@QuLogic QuLogic deleted the optional-face branch July 2, 2025 18:51
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

X Tutup