Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tick label font family via tick_params #25746

Merged
merged 2 commits into from May 17, 2023
Merged

Conversation

saranti
Copy link
Contributor

@saranti saranti commented Apr 21, 2023

PR Summary

Closes #18425. Added a labelfont kwarg to change tick label fonts conveniently, without changing the rest of the text.

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

@@ -36,6 +36,7 @@ class Tick(martist.Artist):
pad: float | None = ...,
labelsize: float | None = ...,
labelcolor: ColorType | None = ...,
labelFont: Sequence[str] = ...,
Copy link
Member

Choose a reason for hiding this comment

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

I think you are conflating "font" with "font family" here. These are two different properties with different types expected. The former is an Alias of "fontproperties" and accepts a font_manager.FontProperties object as well as a few other options to pass into the constructor thereof. The latter accepts str | Iterable[str]

Both behave equivalently when passed a str, as in your test, but font also allows setting other attributes of the font, such as size and color.

Also your indentation and capitalization are not consistent here.
And this is (surprisingly) the first reference to Sequence (or Iterable) in this file so whichever is used (if kept) should be imported from collections.abc.

Copy link
Member

Choose a reason for hiding this comment

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

I think for consistency, it should likely be spelled out that this is setting the labelfontfamily (if that is the type you expect to pass) or the labelfontproperties (as it is implemented, in which case the type hint should match that underlying Text setter).

Further, though if you do keep it as labelfontfamily, it does need to be type hinted as str | Sequence[str] | None (str because that is what you actually pass in the test, None because None is the default)

@@ -175,11 +175,11 @@ def __init__(
self.label1 = mtext.Text(
np.nan, np.nan,
fontsize=labelsize, color=labelcolor, visible=label1On,
fontproperties=labelfont, rotation=self._labelrotation[1])
fontproperties=labelfontfamily, rotation=self._labelrotation[1])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fontproperties=labelfontfamily, rotation=self._labelrotation[1])
fontfamily=labelfontfamily, rotation=self._labelrotation[1])

self.label2 = mtext.Text(
np.nan, np.nan,
fontsize=labelsize, color=labelcolor, visible=label2On,
fontproperties=labelfont, rotation=self._labelrotation[1])
fontproperties=labelfontfamily, rotation=self._labelrotation[1])
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fontproperties=labelfontfamily, rotation=self._labelrotation[1])
fontfamily=labelfontfamily, rotation=self._labelrotation[1])


.. code-block:: python

Axis.tick_params(labelfont='monospace')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Axis.tick_params(labelfont='monospace')
Axis.tick_params(labelfontfamily='monospace')

Copy link
Member

@ksunden ksunden left a comment

Choose a reason for hiding this comment

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

Modulo the extra character in the what's new doc

doc/users/next_whats_new/tick_labelfont_param.rst Outdated Show resolved Hide resolved
@timhoffm timhoffm added this to the v3.8.0 milestone May 17, 2023
@timhoffm timhoffm merged commit 2466ccb into matplotlib:main May 17, 2023
34 of 38 checks passed
@saranti saranti deleted the labelfont branch June 23, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add fontfamily/labelfont to tick_params
4 participants