write addfont example #24866

saranti · 2023-01-02T06:14:16Z

PR Summary

Fixes #24419. Adds an example to the addfont function in the fontManager reference docs.

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

story645

thanks for tackling this - sorry I didn't get back to you, but I don't know much about fonts either.

story645 · 2023-01-02T07:47:46Z

lib/matplotlib/font_manager.py

+            for font_file in font_files:
+            font_manager.fontManager.addfont(font_file)


needs indent

story645 · 2023-01-02T07:48:27Z

lib/matplotlib/font_manager.py

+
+        `addfont` must be called on the global `fontManager` instance::
+
+            import matplotlib.pyplot as plt


so I think since this is API reference, you can drop the plotting example

No worries. I added the plot because i thought you wanted to show how it works in the example.

I can honestly go either way when it's a parameter docstring so now I'm leaning again towards keeping it so folks know how to test if they did it correctly 😅

saranti · 2023-01-03T09:47:23Z

doc/gallery/text_labels_and_annotations/font_family_rc.py

+or a combination of both.
+(see :doc:`font tutorial </tutorials/text/text_props>`)
+
+See `~.font_manager.FontManager.addfont` for adding a font family from a file.


That's a weird one. I only meant to add line 12

tacaswell · 2023-01-04T16:19:02Z

I continue to be extremely skeptical that we want to document this or "here is how to do something we think is a really bad idea".

If we really do want to document this (which again I have doubts about), it should go in the comment at the top of https://matplotlib.org/stable/api/font_manager_api.html#module-matplotlib.font_manager or in the docstring for https://matplotlib.org/stable/api/font_manager_api.html#matplotlib.font_manager.fontManager

anntzer · 2023-01-04T16:34:00Z

@tacaswell Do you mean you think addfont() should not have been public API? See discussion at #15068.

tacaswell · 2023-01-04T17:13:15Z

Not fair to quote past me against my current me!

My thinking has moved more towards "we should not roll our own font discovery" given that we have gotten pushed that way (and there are system APIs for doing it) on windows and osx. Saying "put the fonts where your system expects" and then rebuilding our cache seems like both the correct and easiest (for us) path forward.

That said, addFont has been around for a while and is a pragmatic solution (I have been thinking about packaging in general recently and hence am very down on pragmatic solutions that could be abused because they will be abused in ways that will cause great pain) so I am not in favor of ripping it out at this point. If it were proposed today I would be weakly against it.

On a bit more consideration, that addFont does not modify the cache seems correct to avoid some really confusing action-at-a-distance issues with runtime state getting latched too strongly (if you want to use addFont you have to call it every process).

I think the information that needs to be clear:

the FontManager instance that matters is the singleton at mpl.font_manager.fontmanager
how to rebuild the cache
addFont does not update the cache

but we should not suggest things (abusing pypi to distribute fonts (which we ourselves do)) that we think are a bad idea!

story645 · 2023-01-05T00:40:04Z

it should go in the comment at the top of https://matplotlib.org/stable/api/font_manager_api.html#module-matplotlib.font_manager

I'd pull that out of a comment into docs since it seems to be a summary of the module, but I think adding this info to the top makes sense. Maybe w/ a nice big note section w/ all the caveats. I was thinking something in the style of the animation or color docs.

saranti · 2023-01-06T06:06:39Z

Maybe I should put a bookmark in this issue for the time being.

jklymak · 2023-01-12T21:22:19Z

@saranti. We discussed this at the weekly meeting. I don't personally understand all the ins and outs, so I won't suggest wording, but the consensus was that the addfont doctoring should have a couple sentences of explanation, and then a link to a longer explanation in font_manager.py API docs.

I believe some of the technical details that have to get across are that addfont does not add the font to our font cache, and hence needs to be rendered each time the script is run, and hence folks should only use this as an escape route if they cannot properly install a font on their machine.

saranti · 2023-01-14T04:34:56Z

@saranti. We discussed this at the weekly meeting. I don't personally understand all the ins and outs, so I won't suggest wording, but the consensus was that the addfont doctoring should have a couple sentences of explanation, and then a link to a longer explanation in font_manager.py API docs.

I believe some of the technical details that have to get across are that addfont does not add the font to our font cache, and hence needs to be rendered each time the script is run, and hence folks should only use this as an escape route if they cannot properly install a font on their machine.

Would it be a good idea to add the longer explanation with an example in the text tutorials, just below Default Font?

…addfont

story645

Nit about wording but solves my documentation concerns.

story645 · 2023-01-29T03:20:57Z

lib/matplotlib/font_manager.py

+    However, fonts added with the `FontManager.addfont` method will not persist
+    in the cache and hence will need to be called at every process. This method


Suggested change

However, fonts added with the `FontManager.addfont` method will not persist

in the cache and hence will need to be called at every process. This method

Fonts added with the `FontManager.addfont` method will not persist

in the cache; therefore *addfont* will need to be called every time matplotlib is imported. This method

not quite sure, but something a little more explicit

jklymak · 2023-01-31T00:40:31Z

Thanks @saranti - I took the liberty of squash merging....

write addfont example

13453ea

story645 reviewed Jan 2, 2023

View reviewed changes

fix indent

bd49d5a

saranti commented Jan 3, 2023

View reviewed changes

story645 mentioned this pull request Jan 3, 2023

[Doc]: Documentation should clearly describe how a user can make matplotlib load custom fonts per default #24872

Open

saranti force-pushed the addfont branch from b7b9715 to bd49d5a Compare January 4, 2023 10:11

melissawm added topic: text/fonts Documentation status: needs comment/discussion needs consensus on next step labels Jan 6, 2023

saranti added 4 commits January 21, 2023 18:07

Merge branch 'main' of https://github.com/matplotlib/matplotlib into …

bedb798

…addfont

update addfont example

4a81dd9

cleanup

d96c4ea

remove line

cdb925f

story645 approved these changes Jan 29, 2023

View reviewed changes

add hanah's suggestion

7f798e0

jklymak merged commit f2100d5 into matplotlib:main Jan 31, 2023

saranti deleted the addfont branch February 14, 2023 08:25

QuLogic added this to the v3.8.0 milestone Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

write addfont example #24866

write addfont example #24866

saranti commented Jan 2, 2023

story645 left a comment

story645 Jan 2, 2023

story645 Jan 2, 2023

saranti Jan 2, 2023

story645 Jan 2, 2023

saranti Jan 3, 2023 •

edited

Loading

tacaswell commented Jan 4, 2023

anntzer commented Jan 4, 2023

tacaswell commented Jan 4, 2023

story645 commented Jan 5, 2023

saranti commented Jan 6, 2023

jklymak commented Jan 12, 2023

saranti commented Jan 14, 2023

story645 left a comment

story645 Jan 29, 2023

jklymak commented Jan 31, 2023

		for font_file in font_files:
		font_manager.fontManager.addfont(font_file)


		`addfont` must be called on the global `fontManager` instance::

		import matplotlib.pyplot as plt

		However, fonts added with the `FontManager.addfont` method will not persist
		in the cache and hence will need to be called at every process. This method

write addfont example #24866

write addfont example #24866

Conversation

saranti commented Jan 2, 2023

PR Summary

PR Checklist

story645 left a comment

Choose a reason for hiding this comment

story645 Jan 2, 2023

Choose a reason for hiding this comment

story645 Jan 2, 2023

Choose a reason for hiding this comment

saranti Jan 2, 2023

Choose a reason for hiding this comment

story645 Jan 2, 2023

Choose a reason for hiding this comment

saranti Jan 3, 2023 • edited Loading

Choose a reason for hiding this comment

tacaswell commented Jan 4, 2023

anntzer commented Jan 4, 2023

tacaswell commented Jan 4, 2023

story645 commented Jan 5, 2023

saranti commented Jan 6, 2023

jklymak commented Jan 12, 2023

saranti commented Jan 14, 2023

story645 left a comment

Choose a reason for hiding this comment

story645 Jan 29, 2023

Choose a reason for hiding this comment

jklymak commented Jan 31, 2023

saranti Jan 3, 2023 •

edited

Loading