The PR needs a bit of the usual polishing (which I'll let others walk you through ;-)), but I think attaching that info to the FontProperties object (rather than as a separate attribute on the Text) is a really nice idea that makes things much simpler than I would have feared -- from a quick look, I think the basic approach is good.

All the tests that I could do have been done and passed =D
I think the changes are on a good state to be reviewed now.

I have no firm idea what this new kwarg is doing. What happens if I don't set this kwarg?

If you dont set this kwarg the mathtext will be rendered with the value specified inrcParams['mathtext.fontset']

What even is a "fontset"?

I might be just as lost as you, but I remeber reading somewhere in the docs that a "fontset" is a family of fonts. I will check again to be sure.

Is this the same? Isfontset just an alias formathtext_fontset? Why?

I just made this for convenience, there are a lot of aliases for the parameters to create a Text likesize andfontsize, while studying the creation of a Text class I saw how easy it was to make an alias and, since the namemathtext_fontset seemed too long, I made it. I can revert if you dont think it is necessary.

@DiegoLeal They were more rhetorical questions than suggestions that you change anything, though we may want to do so.

If I understand correctly then I would explain it like this: Matplotlib has a built in math formatter that uses :rc:text.mathtext_fontset to decide what font family to use, and this font family is distinct from the font family used for plain text. This PR allows this "fontset" to be changed on a text-by-text basis, rather than relying on a global value.

I think the todos are:

• decide if we want the alias - I'd say no, because it sounds like it applies to non-math fonts as well. If you wanter to go call itmath_font I wouldn't object to that.
• Document the new kwarg inclass FontProperties
• Expand on the example by explicitly showing a) that the kwargdoesn't change non-math fonts and b) compare using the rcParam and show how the kwarg overrides the rcParam.

BTW, please feel free to ping me for a re-review. PRs sometimes get buried, and politely asking for us to take a look again is encouraged (within reason ;-)

@DiegoLeal Sorry if I was unclear above - I was only suggestingmath_font as analias instead offontset. I think I agree with your original choice that all the methods should be calledset/get/foo_mathtext_fontset just to be compatible with the existing rcParam.

Ups! =D
I will put it back, indeed themathtext_fontset looks more comprehensible.

@jklymak I believe the code is in a good state for a review, please take a look when you can :)

Here's a quick preview of what I did:

• Improved (a lot!) the example (btw, the idea I had of an example was completely off, I didn't even knew they were integrated to the docs, I am sorry to have wasted your time by making you read through that monstrosity, it should be reasonable now)

• Put back themathtext_fontset param, also made it a getter/setter in thefontProperties. ( Couldn't make it get/set in theText though, there's a function there that looks for a specificset_foo parameter for every property and will throw a error if it finds none. May I inquire if this was an intentional decision or maybe python just didn't support setters in the past and you had to work around?)

• If you dont set anymathtext_fontset param, it will render all text in the last set value ofrcParams just as before. (the example has a clear demonstration of this).

I wonder if we should go withmath_font (ormath_fontfamily) overall.mathtext_fontset feels very bulky. The "text" part is unnecessary because text is implied by "font". I haven't found a strong indication that "fontset" is widely used (e.g.https://en.wikibooks.org/wiki/LaTeX/Fonts does not use the term). It was introduced in8dc31750 without much explanation.

This would create a naming difference between thercParam and the property. But now that we're expanding the feature, it may be the right time to turn around and cleanup the naming.

I wonder if we should go withmath_font (ormath_fontfamily) overall...
...This would create a naming difference between the rcParam and the property

I think this trade off is worth. I will wait some time before making this changes to give people time for a response and avoid having to rename the files again in case we decide for another name.

Also, thanks everyone for your reviews and your time I hope my changes, albeit simple, makematplotlib even better :D

Shouldn't this
git rebase -i issue#17906/master
be
git rebase -i upstream/master

I do a git log and rebase versus the last commit I didn't author as part of the PR.

git rebase -i issue#17906/master*squashed the commits here*git push --force origin master

issue#17906 is the name of your branch, andmaster is the name of another branch. Putting them together doesn't name a revision. As@timhoffm said, you should rebase your branch onupstream/master. You also need to force push your branch, notmaster, which you haven't change and is why git says "everything up-to-date".

It looks like you squashed some commits, but didn't do a rebase (checkgit log --graph issue#17906.) Starting after38c362b, you have one line (ae2a558..682a848) with squashed commits, and one line (5c84526..6f0a9da) with the original commits, and then they're merged together ate7f485b.

To reset to the squashed version of commits:

$ git checkout issue#17906$ git reset --hard 682a84866774a168c9d5edcfc64a7f44f58c5fa7

then rebase to current:

$ git fetch upstream$ git rebase -i upstream/master# change squashing, if you want, but it's already done from the first time you did it

You will get stuck somewhere in between because of the conflicts:

$$EDITOR lib/matplotlib/mathtext.py# fix the conflict here, which has to do with a parameter rename in `_parse_cached`$ git add lib/matplotlib/mathtext.py$ git rebase --continue

then force push away the merge, and the other set of commits:

$ git push --force origin issue#17906

I think I finally understand (at least part of it), thank you so much@timhoffm and@QuLogic , after countless hours reading docs and tutorials and trying without success to make a repo for testing I was starting to lose hope, I can not thank you enough. I feel reinvigorated to read a bit more about this stuff now, maybe git is not all the bad things I called it in my thoughts. Thank you very much! :)

It seems some pytest errors appeared after I solved the conflict with_force_standard_ps_fonts. The original PR associated with the conflict is closed and all tests have passed (#18002) so my changes must be breaking them somehow.

I started investigating this but have yet to finish.

As I suspected, the errors seems to persist even after I reset my changes (I verified and the errors are the same).
What should I do? Wait untill it gets corrected to test again?

You can ignore that macOS failure.

@DiegoLeal sorry this is such a pain! OTOH, I think you have an empty commit - i.e. 0 files changed now.

@dopplershift thanks for the info!
@jklymak I had reverted the changes to check that the problem was not due to them, I've reset the revertion, they should be back, sorry!

I've put everything in a single commit because I see that's the way it seems to be done by others, I left messages for the big implementations and deleted messages related to debugs and discussion.

Done and done! I also changed the font a little on the tests to make them more distinguishable. The size of the of the baseline image fell from 8 to 5kb, excellent suggestion QuLogic!

Thanks for all the hard work@diegopetrola

Thanks for all your reviews and guidance@jklymak. You and all the other reviewers who participated here. It was a pleasure.

Hello, I'm the author of the feature request, and I thank the@jklymak and all for making this PR happen. However, under matplotlib 3.3.2

In [5]: matplotlib.__version__Out[5]: '3.3.2'

I cannot run the example

"""===============Math fontfamily===============A simple example showcasing the new *math_fontfamily* parameter that canbe used to change the family of fonts for each individual textelement in a plot.If no parameter is set, the global value:rc:`mathtext.fontset` will be used."""import matplotlib.pyplot as pltplt.figure(figsize=(6, 5))# A simple plot for the background.plt.plot(range(11), color="0.9")# A text mixing normal text and math text.msg = (r"Normal Text. $Text\ in\ math\ mode:\ "       r"\int_{0}^{\infty } x^2 dx$")# Set the text in the plot.plt.text(1, 7, msg, size=12, math_fontfamily='cm')# Set another font for the next text.plt.text(1, 3, msg, size=12, math_fontfamily='dejavuserif')# *math_fontfamily* can be used in most places where there is text,# like in the title:plt.title(r"$Title\ in\ math\ mode:\ \int_{0}^{\infty } x^2 dx$",          math_fontfamily='stixsans', size=14)# Note that the normal text is not changed by *math_fontfamily*.plt.show()

errors saying thatmath_fontfamily is not supported

Nevermind please, I see it is under 3.4 milestone. I wonder when can we approximately expect it to be released?

@diegopetrola
Hi, I recently suffer from the issue caused bymath_fontfamily.

If I set the font in text explicitly, then I have to setmath_fontfamily as well, otherwise,math_fontfamily will beNone and cause exceptions. (even if I set a global default inrcParams['mathtext.fontset'])

Here is a simple reproduce snippet: (it would be working well if you pass the argumentmath_fontfamily='cm').
I think there should be a default setting if I don't passmath_fontfamily to the Text, and usingrcParams['mathtext.fontset'] seems good.

importmatplotlib.pyplotaspltimportmatplotlibmatplotlib.rcParams['mathtext.fontset']='cm'plt.figure(figsize=(6,5))plt.plot(range(11),color="0.9")msg= (r"Normal Text. $Text\ in\ math\ mode:\ "r"\int_{0}^{\infty } x^2 dx$")plt.text(1,7,msg,size=12,font='Arial')# math_fontfamily='cm'plt.show()

I check the error and it seems that, theArial FontProperties passed to that Text is constructed withmath_fontfamily=None.

Error log:

Exception in Tkinter callbackTraceback (most recent call last):  File "C:\Users\ain-s\miniconda3\lib\tkinter\__init__.py", line 1892, in __call__    return self.func(*args)  File "C:\Users\ain-s\miniconda3\lib\tkinter\__init__.py", line 814, in callit    func(*args)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\backends\_backend_tk.py", line 239, in idle_draw    self.draw()  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 9, in draw    super().draw()  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\backends\backend_agg.py", line 406, in draw    self.figure.draw(self.renderer)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\artist.py", line 73, in draw_wrapper    result = draw(artist, renderer, *args, **kwargs)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\artist.py", line 50, in draw_wrapper    return draw(artist, renderer, *args, **kwargs)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\figure.py", line 2737, in draw    mimage._draw_list_compositing_images(  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\image.py", line 132, in _draw_list_compositing_images    a.draw(renderer)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\artist.py", line 50, in draw_wrapper    return draw(artist, renderer, *args, **kwargs)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\_api\deprecation.py", line 431, in wrapper    return func(*inner_args, **inner_kwargs)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\axes\_base.py", line 2925, in draw    mimage._draw_list_compositing_images(renderer, self, artists)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\image.py", line 132, in _draw_list_compositing_images    a.draw(renderer)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\artist.py", line 50, in draw_wrapper    return draw(artist, renderer, *args, **kwargs)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\text.py", line 679, in draw    bbox, info, descent = textobj._get_layout(renderer)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\text.py", line 314, in _get_layout    w, h, d = renderer.get_text_width_height_descent(  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\backends\backend_agg.py", line 235, in get_text_width_height_descent    self.mathtext_parser.parse(s, self.dpi, prop)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\mathtext.py", line 452, in parse    return self._parse_cached(s, dpi, prop, _force_standard_ps_fonts)  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\mathtext.py", line 461, in _parse_cached    else _api.check_getitem(  File "C:\Users\ain-s\miniconda3\lib\site-packages\matplotlib\_api\__init__.py", line 189, in check_getitem    raise ValueError(ValueError: None is not a valid value for fontset; supported values are 'cm', 'dejavuserif', 'dejavusans', 'stix', 'stixsans', 'custom'

Please open a separate issue if this is a problem in the current release.