Skip to content

Commit

Permalink
MNT: Cleanup FontProperties __init__ API
Browse files Browse the repository at this point in the history
The cleanup approach is to not modify
code logic during a deprecation period,
but only detect deprecated call patterns
through a decorator and warn on them.
  • Loading branch information
timhoffm committed Sep 20, 2024
1 parent 5a0cdf1 commit 73cbd3d
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 4 deletions.
9 changes: 9 additions & 0 deletions doc/api/next_api_changes/deprecations/28843-TH.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FontProperties initialization
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

`.FontProperties` initialization is limited to the two call patterns:

- single positional parameter, interpreted as fontconfig pattern
- only keyword parameters for setting individual properties

All other previously supported call patterns are deprecated.
64 changes: 61 additions & 3 deletions lib/matplotlib/font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@

from base64 import b64encode
from collections import namedtuple
from collections.abc import Iterable
import copy
import dataclasses
from functools import lru_cache
import functools
from io import BytesIO
import json
import logging
Expand Down Expand Up @@ -536,6 +538,56 @@ def afmFontProperty(fontpath, font):
return FontEntry(fontpath, name, style, variant, weight, stretch, size)


def _cleanup_fontproperties_init(init_method):
"""
A decorator to limit the call signature to single a positional argument
or alternatively only keyword arguments.
We still accept but deprecate all other call signatures.
When the deprecation expires we can switch the signature to::
__init__(self, pattern=None, /, *, family=None, style=None, ...)
plus a runtime check that pattern is not used alongside with the
keyword arguments. This results eventually in the two possible
call signatures::
FontProperties(pattern)
FontProperties(family=..., size=..., ...)
"""
@functools.wraps(init_method)
def wrapper(self, *args, **kwargs):
# multiple args with at least some positional ones
if len(args) > 1 or len(args) == 1 and kwargs:
# Note: Both cases were previously handled as individual properties.
# Therefore, we do not mention the case of font properties here.
_api.warn_deprecated(
"3.10",
message="Passing individual properties to FontProperties() "
"positionally is deprecated. Please pass all properties "
"via keyword arguments."
)
# single non-string arg -> clearly a family not a pattern
if (len(args) == 1 and not kwargs
and isinstance(args[0], Iterable) and not isinstance(args[0], str)):
# Case font-family list passed as single argument
_api.warn_deprecated(
"3.10",
message="Passing family as positional argument to FontProperties() "
"is deprecated. Please pass family names as keyword"
"argument."
)
# Note on single string arg:
# This has been interpreted as pattern so far. We are already raising if a
# non-pattern compatible family string was given. Therefore, we do not need
# to warn for this case.
return init_method(self, *args, **kwargs)

return wrapper


class FontProperties:
"""
A class for storing and manipulating font properties.
Expand Down Expand Up @@ -585,9 +637,14 @@ class FontProperties:
approach allows all text sizes to be made larger or smaller based
on the font manager's default font size.
This class will also accept a fontconfig_ pattern_, if it is the only
argument provided. This support does not depend on fontconfig; we are
merely borrowing its pattern syntax for use here.
This class accepts a single positional string as fontconfig_ pattern_,
or alternatively individual properties as keyword arguments::
FontProperties(pattern)
FontProperties(*, family=None, style=None, variant=None, ...)
This support does not depend on fontconfig; we are merely borrowing its
pattern syntax for use here.
.. _fontconfig: https://www.freedesktop.org/wiki/Software/fontconfig/
.. _pattern:
Expand All @@ -599,6 +656,7 @@ class FontProperties:
fontconfig.
"""

@_cleanup_fontproperties_init
def __init__(self, family=None, style=None, variant=None, weight=None,
stretch=None, size=None,
fname=None, # if set, it's a hardcoded filename to use
Expand Down
40 changes: 40 additions & 0 deletions lib/matplotlib/tests/test_font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import numpy as np
import pytest

import matplotlib as mpl
from matplotlib.font_manager import (
findfont, findSystemFonts, FontEntry, FontProperties, fontManager,
json_dump, json_load, get_font, is_opentype_cff_font,
Expand Down Expand Up @@ -367,3 +368,42 @@ def inner():
for obj in gc.get_objects():
if isinstance(obj, SomeObject):
pytest.fail("object from inner stack still alive")


def test_fontproperties_init_deprecation():
"""
Test the deprecated API of FontProperties.__init__.
The deprecation does not change behavior, it only adds a deprecation warning
via a decorator. Therefore, the purpose of this test is limited to check
which calls do and do not issue deprecation warnings. Behavior is still
tested via the existing regular tests.
"""
with pytest.warns(mpl.MatplotlibDeprecationWarning):
# multiple positional arguments
FontProperties("Times", "italic")

with pytest.warns(mpl.MatplotlibDeprecationWarning):
# Mixed positional and keyword arguments
FontProperties("Times", size=10)

with pytest.warns(mpl.MatplotlibDeprecationWarning):
# passing a family list positionally
FontProperties(["Times"])

# still accepted:
FontProperties(family="Times", style="italic")
FontProperties(family="Times")
FontProperties("Times") # works as pattern and family
FontProperties("serif-24:style=oblique:weight=bold") # pattern

# also still accepted:
# passing as pattern via family kwarg was not covered by the docs but
# historically worked. This is left unchanged for now.
# AFAICT, we cannot detect this: We can determine whether a string
# works as pattern, but that doesn't help, because there are strings
# that are both pattern and family. We would need to identify, whether
# a string is *not* a valid family.
# Since this case is not covered by docs, I've refrained from jumping
# extra hoops to detect this possible API misuse.
FontProperties(family="serif-24:style=oblique:weight=bold")
2 changes: 1 addition & 1 deletion lib/matplotlib/ticker.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ def set_useMathText(self, val):
from matplotlib import font_manager
ufont = font_manager.findfont(
font_manager.FontProperties(
mpl.rcParams["font.family"]
family=mpl.rcParams["font.family"]
),
fallback_to_default=False,
)
Expand Down

0 comments on commit 73cbd3d

Please sign in to comment.