Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add feature to fallback to stix font in mathtext #11644

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

Closed

Conversation

Copy link

@nelsond nelsond commented Jul 12, 2018

PR Summary

When using a custom font for math (mathtext.fontset : custom), the only fallback for undefined symbols/characters is Computer Modern at the moment. See also #8619.

The pull request contains a new rcParam mathtext.fallback which can be stix, cm or None.

mathtext.fallback: stix

Support for the original rcParam mathtext.fallback_to_cm is still available. The feature seems to work as the plots shows.

stix cm

(Example plot with STIX (left) respectively CM (right) as mathtext fallback and Helvetica Neue as custom mathtext font)

If this pull request has any chance of getting merged, I'm happy to clean up the code, add documentation, and testing.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

himbeles and andrzejnovak reacted with thumbs up emoji
- Add new rcParam `mathtext.fallback`
- Update UnicodeFonts class to support fallback to stix
Copy link

Hi,
seems to be working nicely for me except for "scaled" brackets using \left and \right TeX syntax with stix.
Minimum failing and working examples:

import matplotlib as mpl
import matplotlib.pyplot as plt
# MATH FONT
mpl.rcParams['mathtext.fontset'] = 'custom'
mpl.rcParams['mathtext.rm'] = 'Helvetica Neue'
mpl.rcParams['mathtext.it'] = 'Helvetica Neue:italic'
mpl.rcParams['mathtext.bf'] = 'Helvetica Neue:bold'
test_str1 = r'$\left(a\right) \rightarrow 1$'
test_str2 = r'$a \rightarrow 1$'
def test_plot(test_str):
 fig = plt.figure(figsize=(7,5))
 plt.title(mpl.rcParams['mathtext.fallback'])
 plt.plot([0,2],[0,2])
 plt.text(1,1,test_str, fontsize=20)
 plt.show()
# working example 1
mpl.rcParams['mathtext.fallback'] = "cm"
test_plot(test_str1)
# working example 1
mpl.rcParams['mathtext.fallback'] = "stix"
test_plot(test_str2)
# failing example
mpl.rcParams['mathtext.fallback'] = "stix"
test_plot(test_str1)

Copy link
Author

nelsond commented Jul 12, 2018

@himbeles Thanks for pointing this out. The last commit (3ae26b7) should fix this issue. Your example now works for me, see below.

stix sized alternatives

himbeles reacted with thumbs up emoji

Copy link

@nelsond 👍 Working great, thank you!

@tacaswell tacaswell added this to the v3.1 milestone Jul 12, 2018
elif isinstance(self.cm_fallback, StixFonts):
warnings.warn(
"Substituting with a symbol from STIX.",
MathTextWarning)
if (fontname in ('it', 'regular') and
isinstance(self.cm_fallback, StixFonts)):
return self.cm_fallback._get_glyph(
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 could just set fontname='rm' in this line, get rid of the else block below, and then just have a single return self.cm_fallback._get_glyph(fontname, font_class, sym, fontsize) instead of three.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. I fixed this a couple of days ago.

@@ -771,6 +774,13 @@ def __init__(self, *args, **kwargs):
prop = FontProperties('cmex10')
font = findfont(prop)
self.fontmap['ex'] = font
# include stix sized alternatives for glyphs if fallback is stix
if isinstance(self.cm_fallback, StixFonts):
stixsizedaltfonts = "STIXGeneral STIXSizeOneSym STIXSizeTwoSym "\
Copy link
Member

Choose a reason for hiding this comment

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

(削除) I would directly write this as a list stixsizedaltfonts = ["STIXGeneral", ...].
It's more readable than splitting a string. Also, you can wrap within the list and don't need the ugly line continuation char 😄. (削除ここまで)
See dict suggestion below.

"STIXSizeThreeSym STIXSizeFourSym STIXSizeFiveSym".split()
for i, stixfont in enumerate(stixsizedaltfonts):
font = findfont(stixfont)
self.fontmap[i] = font
Copy link
Member

@timhoffm timhoffm Jul 15, 2018
edited
Loading

Choose a reason for hiding this comment

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

Since the numbering is essential, have you thought about turning stixsizedaltfonts into a dict, similar to what is done in DejaVuFonts?

Copy link
Author

Choose a reason for hiding this comment

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

I went for splitting a string here because that is also done in line 770.

for texfont in "cal rm tt it bf sf".split():
# ...

But of course, there is no line break and the order does not matter. So, I agree that a dict is more appropriate here. I also made sure to add the STIX alternatives with their names to the fontmap

self.fontmap[name] = fullpath

Not sure whether this is really necessary, but it is also the case for the DejaVuFonts.

if s is None:
return s

if isinstance(s, str):
Copy link
Member

@timhoffm timhoffm Jul 15, 2018
edited
Loading

Choose a reason for hiding this comment

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

Valid values of s must be a string. This check can be left out and you can just if s.lower() in fallback_fonts:. If you want to provide a better error message in case the user passes some other object, it's better to explicitly

if not isinstance(s, str):
 raise ValueError("'mathtext.fallback' must be a string.")

Copy link
Author

Choose a reason for hiding this comment

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

I changed this. Not sure, I understand this completely, since validate_fontsize has a similiar check/conversion. But maybe I am missing a subtle point here.

def validate_fontsize(s):
 fontsizes = ['xx-small', 'x-small', 'small', 'medium', 'large',
 'x-large', 'xx-large', 'smaller', 'larger']
 if isinstance(s, str):
 s = s.lower()
 if s in fontsizes:
 return s
 try:
 return float(s)
 except ValueError:
 raise ValueError("%s is not a valid font size. Valid font sizes "
 "are %s." % (s, ", ".join(fontsizes)))

Copy link
Member

Choose a reason for hiding this comment

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

validate_fontsizes does also accept numbers.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I see. But I might want to go with your initial suggestion then. Currently, the following code

mpl.rcParams['mathtext.fallback'] = 1

leads to the error

AttributeError: 'int' object has no attribute 'lower'

Of course the user should only supply a string, but the error message might not be very helpful in finding a potential bug in your own code accessing mpl.rcParams['mathtext.fallback'].

I added the following

if not isinstance(s, str):
 raise ValueError("Must be a string or None.")

which yields the following error message

ValueError: Key mathtext.fallback: Must be a string or None.

I hope that's ok.

@@ -1117,6 +1131,7 @@ def _validate_linestyle(ls):
'mathtext.fontset': ['dejavusans', validate_fontset],
'mathtext.default': ['it', validate_mathtext_default],
'mathtext.fallback_to_cm': [True, validate_bool],
'mathtext.fallback': [None, validate_mathtext_fallback],
Copy link
Member

Choose a reason for hiding this comment

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

This needs some documentation. In particular it seems fallback overrides fallback_to_cm if its set; there should be a comment in here.

Preferably, we would deprecate fallback_to_cm and make cm the default for mathtext.fallback.

This needs to be in matplotlibrc.template as well, right?

anntzer reacted with thumbs up emoji
@@ -772,6 +775,21 @@ def __init__(self, *args, **kwargs):
font = findfont(prop)
self.fontmap['ex'] = font

# include STIZ sized alternatives for glyphs if fallback is STIX
Copy link
Contributor

@anntzer anntzer Nov 7, 2018

Choose a reason for hiding this comment

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

STIX? (typo)

Copy link
Contributor

Sorry about opening #16492. Having this up would be great :)

Copy link
Contributor

anntzer commented Feb 13, 2020

@andrzejnovak Do you think this should also support fallback = dejavusans/... (basically anything supported by mathtext.fontset) as suggested in #8619? Would you like to pick up this PR and manage the rebase and push it through the finish line? I think there are the review comments to address, at least a little bit of doc (it's pretty obscure...), perhaps deprecate fallback_to_cm?, and possibly support the other fallbacks as well.

Copy link
Contributor

@anntzer Seems what's missing is largely cosmetic changes, I could take a look. In principle, this should imo support any fallback font if available, such as Fira Math. But the current scope would already be enough of a fix for me.

Copy link
Contributor

anntzer commented Feb 13, 2020

Go for it :)

Copy link
Member

QuLogic commented Mar 10, 2020

This is replaced by #16509, isn't it?

Copy link
Contributor

anntzer commented Mar 10, 2020

AFAICT, yes.
Thanks for the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anntzer anntzer anntzer left review comments

@jklymak jklymak jklymak left review comments

@dstansby dstansby dstansby left review comments

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /