MNT: Cleanup FontProperties __init__ API

timhoffm · timhoffm · commit 73cbd3dc6b44 · 2024-09-20T09:59:37.000+02:00
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.
diff --git a/doc/api/next_api_changes/deprecations/28843-TH.rst b/doc/api/next_api_changes/deprecations/28843-TH.rst
@@ -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.
diff --git a/lib/matplotlib/font_manager.py b/lib/matplotlib/font_manager.py
@@ -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
@@ -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.
@@ -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:
@@ -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
diff --git a/lib/matplotlib/tests/test_font_manager.py b/lib/matplotlib/tests/test_font_manager.py
@@ -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,
@@ -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")
diff --git a/lib/matplotlib/ticker.py b/lib/matplotlib/ticker.py
@@ -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,
                     )

Original file line number	Diff line number	Diff line change
`@@ -574,7 +574,7 @@ def set_useMathText(self, val):`
`574`	`574`	`from matplotlib import font_manager`
`575`	`575`	`ufont = font_manager.findfont(`
`576`	`576`	`font_manager.FontProperties(`
`577`		`- mpl.rcParams["font.family"]`
	`577`	`+ family=mpl.rcParams["font.family"]`
`578`	`578`	`),`
`579`	`579`	`fallback_to_default=False,`
`580`	`580`	`)`