Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FT2Font extension improvements#28842
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
If you want real Pythonic enums (that actually inherit from enum.Enum) you can reuse the P11X_DECLARE_ENUM macro I wrote in mplcairo. |
2217d5f
toa36f9c0
CompareI can't say I understand all of that macro, but I can certainly copy it over if that's the way we want to go. |
(I don't really mind either way, but you mentioned that real enums may be better, so I suggested it. I can also try to explain better how it works...) |
a36f9c0
toee7082b
CompareQuLogic commentedSep 20, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The failure in the docs actually shows the confusion with the "just a bunch of constants" approach: matplotlib/galleries/examples/misc/ftface_props.py Lines 49 to 63 in9675122
Only Bold and Italic are style flags; the rest are face flags, and checking them as they are here would always return False. Also, given that the enum values are bitfields already, not bit positions, the math there doesn't make sense. |
On second reading, I have some understanding of it all. It would be nice though if I could add a docstring to each class (as done here), and avoid the C++ enum (but I think I need this one for the |
ee7082b
to082b797
CompareI guess to add a docstring you need to do something like |
082b797
to6a3b9fe
Compare
Yep, looks like that worked nicely.
I started making a copy without the |
ee46b09
tod3b6c9d
CompareThe following constants are now part of `.ft2font.Kerning`: | ||
- ``KERNING_DEFAULT`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
perhaps note that the new name doesn't include KERNING_; ditto for LOAD_.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Done.
src/_enums.h Outdated
auto const& [py_base_cls, pairs] = | ||
spec.cast<std::pair<std::string, py::object>>(); | ||
mod.attr(py::cast(py_name)) = spec = | ||
py::module::import("pydoc").attr("locate")(py_base_cls)( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You can probably use pkgutil.resolve_name here instead of the older and undocumented pydoc.locate (though both would work).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Or should we just hard-code this coming fromenum
(and only provide the class name to the macro)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That's probably fine too, anything works here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK, I just did the hard-coding then, and added more explicit docs about it.
ed46569
to016a7b4
CompareAh, apparently using |
016a7b4
to7fd09b7
CompareIf I've understood the remaining failure, MSVC doesn't expand |
You likely need to pass |
OK, I checked and |
c7d1d03
to437dfc0
CompareSo this is failing on AppVeyor because we're still on the 2017 image; I opened#28869 separately to see if we can bump to the 2019 one. |
437dfc0
to52effce
Compare52effce
to3e73e48
CompareWe're passing now that AppVeyor has been updated. |
2096a3a
to69c873d
Compare69c873d
to200b849
Compare200b849
tobc2b853
ComparePort things to numpydoc, and add additional explanation where possible.
This follows some of the idea frommatplotlib#9763, but is based on code frommplcairo.Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
Unfortunately, FreeType doesn't define this as an enum of any sort, justas a bunch of macro `#define`s. So we have to make our own for pybind11to bind.
bc2b853
to5864017
Comparesorry, took two tries to get the rebase right. |
1f0ddbe
intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
PR summary
This does two things:
ft2font
, and follow numpydoc style.Enum
-like objects.Unfortunately,This is based on the implementation inNew FreeType wrapper. #9763 and mplcairo, but extended to all the flags as well.pybind11
doesn't support Python's new-ishEnum
class, but it should be a reasonable approximation.The
Enum
change still accepts plain integers, but raises a deprecation warning (as does accessing those constants from the module.)I have not written an API change note in case we don't want to do this.We may also want to mark the deprecation as pending.PR checklist