Visitar URL original
Fix axisartist label font sizes set through Axes by baba9811 · Pull Request #32384 · matplotlib/matplotlib · GitHub
Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/next_api_changes/behavior/32384-KH.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Axisartist axis labels follow the font size set through Axes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Axis labels in `~mpl_toolkits.axisartist` now use the font size set through
`~matplotlib.axes.Axes.set_xlabel` and `~matplotlib.axes.Axes.set_ylabel`.
Previously, these labels kept their initial font size.

A font size set directly on an axisartist label, for example with
``ax.axis["left"].label.set_fontsize(20)``, continues to take precedence.
3 changes: 3 additions & 0 deletions lib/matplotlib/font_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1473,6 +1473,9 @@ def findfont(self, prop, fontext='ttf', directory=None,
for e in rc_params) # Make this hashable.
if directory is None and os.getenv('MPL_IGNORE_SYSTEM_FONTS'):
directory = cbook._get_data_path('fonts')
if isinstance(prop, FontProperties):
# Cache a snapshot, which cannot change or retain a reference artist.
prop = prop.copy()
Comment on lines +1476 to +1479

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not the responsibility of findfont. It belongs in _findfont_cached.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The snapshot must be taken before lru_cache builds its key. Copying inside the decorated function still retains the original, reference-bearing object; I reproduced the figure remaining alive until the cache was cleared.

I kept this in findfont, the sole production caller, which already prepares the other cache-key inputs. An uncached private wrapper would also be correct, but would add another method and cache-invalidation changes without changing the supported behavior.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair point. It's a bit unfortunate that this is needed outside the cache, but the FontProperties conversion (prop = FontProperties._from_any(prop)) is inside, which we want to keep for speed.

I guess we have to live with the compromize of split responsibility.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I looked into the wrapper option a bit more and tried a small uncached wrapper around _findfont_cached. It keeps the snapshot before cache-key creation and the _from_any conversion inside the cached worker. Cache invalidation can also stay unchanged, so my earlier comment overstated the changes needed.
This still splits the two steps, but keeps the snapshot handling next to the cache implementation rather than in findfont. Would you prefer that small separation, or keeping the current placement?

ret = self._findfont_cached(
prop, fontext, directory, fallback_to_default, rebuild_if_missing,
rc_params)
Expand Down
34 changes: 34 additions & 0 deletions lib/mpl_toolkits/axisartist/axis_artist.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import matplotlib.colors as mcolors
import matplotlib.text as mtext
from matplotlib.collections import LineCollection
from matplotlib.font_manager import FontProperties
from matplotlib.lines import Line2D
from matplotlib.patches import PathPatch
from matplotlib.path import Path
Expand Down Expand Up @@ -298,6 +299,36 @@ def get_window_extent(self, renderer=None):
return bbox


class _AxisLabelFontProperties(FontProperties):
"""Inherit the reference label's size until a size is explicitly set."""

__slots__ = ("_ref_artist",)

def __init__(self, properties, ref_artist):
self.__dict__.update(properties.__dict__)
self._ref_artist = ref_artist

def get_size(self):
if self._ref_artist is not None:
return self._ref_artist.get_fontsize()
return super().get_size()

get_size_in_points = get_size

def set_size(self, size):
super().set_size(size)
self._ref_artist = None

def __copy__(self):
# FontProperties.__copy__ returns a plain FontProperties snapshot.
properties = super().__copy__()
properties.set_size(self.get_size())
return properties

def __hash__(self):
return hash(self.copy())


class AxisLabel(AttributeCopier, LabelBase):
"""
Axis label. Derived from `.Text`. The position of the text is updated
Expand Down Expand Up @@ -1025,6 +1056,9 @@ def _init_label(self, **kwargs):
transform=tr,
axis_direction=self._axis_direction,
)
if "labelsize" not in kwargs:
self.label._fontproperties = _AxisLabelFontProperties(
self.label.get_fontproperties(), self.axis.label)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What motivates the archtectural decision to reference self.axis.label? Have you considered other solution strategies?

If this approach is the right way forward it needs concise documentation what you do (semantically not technically) and why. Also, such a behavior should not be patched onto the label, but should be implmented as capability of the label.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

self.axis.label already supplies the default text and color, so using it for the default font size follows the same behavior. Each axisartist label can still set its own size.

Overriding get_fontsize alone isn't enough here: Text uses _fontproperties directly for layout and drawing. Artist callbacks also don't catch changes made through get_fontproperties().set_size(…). That's why I kept the size lookup in FontProperties.

I agree that AxisLabel should own the setup. The local revision moves it there with set_fontsize("auto") and documents how setting a size or replacing the font properties stops inheritance. TickLabels keeps its existing behavior.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Technically, AxisLabel currently works with self.axis None. However, real use cases should always have self.axis set. If we could rely on that, would it be an option to always delegate font properties lookup to self.axis.label so that we don't have to maintain state in AxisLabel? This is turn would alleviate the need for _AxisLabelFontProperties.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I checked this against main. Left and right labels, as well as floating labels using the same Axis, can currently have independent font settings. Always using the reference label's FontProperties would make their setters modify the same object.

I'd prefer to preserve those local settings for this fix. Did you mean to delegate defaults while keeping local overrides, or to share font settings including edits? Keeping overrides would still require some per-label state, although it need not use _AxisLabelFontProperties.

self.label.set_figure(self.axes.get_figure(root=False))
labelpad = kwargs.get("labelpad", 5)
self.label.set_pad(labelpad)
Expand Down
114 changes: 113 additions & 1 deletion lib/mpl_toolkits/axisartist/tests/test_axis_artist.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import copy
import gc
import weakref

import numpy as np
import pytest

import matplotlib as mpl
import matplotlib.pyplot as plt
from matplotlib.projections import PolarAxes
from matplotlib.testing.decorators import image_comparison
from matplotlib.font_manager import FontManager, FontProperties
from matplotlib.testing.decorators import check_figures_equal, image_comparison
from matplotlib.text import Text
from matplotlib.transforms import Affine2D

from mpl_toolkits.axisartist import (AxisArtistHelperRectlinear, GridHelperCurveLinear,
Expand All @@ -12,6 +19,111 @@
LabelBase, Ticks, TickLabels)


@pytest.mark.parametrize("coordinate, side", [("x", "bottom"), ("y", "left")])
@pytest.mark.parametrize(
"property_name", ["fontsize", "size", "fontdict", "fontproperties"])
@check_figures_equal(extensions=["png", "pdf", "svg"])
def test_label_fontsize_from_axes(fig_test, fig_ref, coordinate, side, property_name):
for fig in [fig_test, fig_ref]:
ax = fig.add_subplot(axes_class=HostAxes)
ax.plot([0, 1], [0, 1])
for size in [20, 14]:
value = ({"fontsize": size} if property_name == "fontdict" else
FontProperties(size=size) if property_name == "fontproperties" else
size)
getattr(ax, f"set_{coordinate}label")(
"Axis label", **{property_name: value})
if fig is fig_ref:
ax.axis[side].label.set_fontsize(size)
fig.canvas.draw()


@pytest.mark.parametrize("method, value, expected", [
("set_fontsize", 20, 20),
("set_fontsize", 30, 30), ("set_size", 30, 30),
("set_fontproperties", FontProperties(size=30), 30),
("set_font", ":size=30", 30),
])
def test_label_fontsize_override(method, value, expected):
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_ylabel("Axis label", fontsize=20)
label = ax.axis["left"].label
getattr(label, method)(value)
ax.set_ylabel("New label", fontsize=14)
fig.canvas.draw()
assert label.get_fontsize() == expected
assert label.get_fontproperties().get_size_in_points() == expected
assert ax.yaxis.label.get_fontsize() == 14


def test_label_fontproperties_mutable():
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_ylabel("Axis label", fontsize=20)
label = ax.axis["left"].label
properties = label.get_fontproperties()
properties.set_weight("bold")
properties.set_size(30)
ax.set_ylabel("New label", fontsize=14)
fig.canvas.draw()
assert label.get_fontproperties() is properties
assert label.get_fontweight() == "bold"
assert label.get_fontsize() == 30


def test_label_fontproperties_update_from():
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_ylabel("Axis label", fontsize=20)
label = ax.axis["left"].label
text = Text()
text.update_from(label)
assert text.get_fontsize() == 20
label.update_from(Text(fontsize=30))
ax.set_ylabel("New label", fontsize=14)
assert label.get_fontsize() == 30


def test_label_fontproperties_copy():
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_ylabel("Axis label", fontsize=20)
properties = ax.axis["left"].label.get_fontproperties()
snapshot = copy.copy(properties)
assert hash(properties) == hash(snapshot)
ax.set_ylabel("New label", fontsize=14)
assert properties.get_size_in_points() == 14
assert snapshot.get_size_in_points() == 20
assert properties != snapshot


def test_label_font_lookup_does_not_keep_figure_alive():
manager = FontManager()
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_xlabel("Axis label", fontsize=20)
manager.findfont(ax.axis["bottom"].label.get_fontproperties())
reference = weakref.ref(fig)
plt.close(fig)
del fig, ax
gc.collect()
assert reference() is None


def test_label_fontsize_bbox():
fig = plt.figure()
ax = fig.add_subplot(axes_class=HostAxes)
ax.set_xlabel("Axis label", fontsize=20)
label = ax.axis["bottom"].label
label.set_bbox({"facecolor": "white"})
fig.canvas.draw()
patch = label.get_bbox_patch()
size = patch.get_width(), patch.get_height()
label.update_bbox_position_size(fig.canvas.get_renderer())
assert (patch.get_width(), patch.get_height()) == size


@image_comparison(['axis_artist_ticks.png'], style='default')
def test_ticks():
fig, ax = plt.subplots()
Expand Down
Loading