[MNT]: Deprecate mutability of FontProperties #22495
Comments
|
Agree with immutability. Interface t.b.d. Could this even be a frozen dataclass? |
|
I think you closed this accidentally? |
|
A frozen dataclass could possibly work, indeed. |
|
Sorry for closing, I hit the wrong button. The change to data classes is a little larger than only removing setters. But it may be simpler to understand and use in the long run. I also don't expect that getters and setters in FontProperties are widely used, so the change could be bearable. |
Replace default initialization immediately followed by setting values by initialization with values, i.e. ``` # before: fp = FontProperties() fp.set_[something](val) # after fp = FontProperties([something]=val) ``` This is clearer and additionally helps with the possible transition of making FontProperties immutable, see matplotlib#22495.
|
The The multi-setter must be the basis and should be written first. Optionally, we can then create |
Replace default initialization immediately followed by setting values by initialization with values, i.e. ``` # before: fp = FontProperties() fp.set_[something](val) # after fp = FontProperties([something]=val) ``` This is clearer and additionally helps with the possible transition of making FontProperties immutable, see matplotlib#22495.
Summary
Currently, FontProperties (FPs) are mutable, which makes caching based on them a bit tricky (#22323, #22487). Manipulation is also not so nice; e.g. the fonts_demo.py example needs to repeatedly copy a base FP instance and call setters on the copies. Moreover, if one writes
then later mutation of
fpdoesn't modify the fontproperties ontextobj, becauseset_fontpropertiesexplicitly makes a copy of its argument. While this kind of "isolate-from-later-changes" is something we typically aim for in Matplotlib (see recent PRs likewise isolating artists from later changes in passed-in data arrays), this also means that there is basically no functionality gain coming from mutable FPs (and at least one complication, in the case of caching). (Ifset_fontpropertiesdid not copy, then mutability would provide a way for two text objects to share exactly the same FP while having to mutate only one of them.)I would therefore argue that the various property setters on FP should be deprecated to ultimately make FPs immutable; instead, we should use
with_foo()methods that return new instances with just a single property being changed (similar to the pathlib API, for example). More precisely, we can either havewith_family(family),with_style(style),with_variant(variant), etc., or instead we could also have a singlewith_props(**kwargs)(called e.g. aswith_props(family="Times", ...)); I don't have a very strong opinion there.Proposed fix
No response
The text was updated successfully, but these errors were encountered: