Skip to content

[Bug]: \, and \mathrm{\,} are not identical in Mathtext when using CM and STIX #23474

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
tfpf opened this issue Jul 22, 2022 · 1 comment · Fixed by #23482
Closed

[Bug]: \, and \mathrm{\,} are not identical in Mathtext when using CM and STIX #23474

tfpf opened this issue Jul 22, 2022 · 1 comment · Fixed by #23482

Comments

@tfpf
Copy link
Contributor

tfpf commented Jul 22, 2022

Bug summary

Title.

Code for reproduction

import matplotlib.pyplot as plt
# plt.rcdefaults()
fig = plt.figure()
for (x, fontset) in enumerate(['cm', 'dejavusans', 'dejavuserif', 'stix', 'stixsans']):
    plt.rc('mathtext', fontset=fontset)
    fig.text(x / 5, .4, r'|$\,$|', size=100, alpha=.4)
    fig.text(x / 5, .6, r'|$\mathrm{\,}$|', size=100, alpha=.4)
    fig.text(x / 5, .9, fontset)
plt.show()

Actual outcome

\, and \mathrm{\,} do not insert the same amount of space.

spacewidths

Expected outcome

\, and \mathrm{\,} should insert the same amount of space. This is what LaTeX does (as far as I can tell by eye).

Additional information

The difference is too minute to matter. As the MWE shows, the text has to be blown up to huge sizes to manifest it. At normal sizes, it is inconsequential. (Case in point: test_mathtext.py::test_operator_space, which compares \sin^2 \cos with \mathrm{sin}^2 \mathrm{\,cos}, thereby indirectly comparing \, and \mathrm{\,}.)

It can have unexpected side-effects, though; see #22852 (comment).

The culprit is the _make_space function. For \,, it returns 1/6 of the advance of an italicised 'm'. For \mathrm{\,}, it returns 1/6 of the advance of a regular 'm'.

def _make_space(self, percentage):
# All spaces are relative to em width
state = self.get_state()
key = (state.font, state.fontsize, state.dpi)
width = self._em_width_cache.get(key)
if width is None:
metrics = state.font_output.get_metrics(
state.font, mpl.rcParams['mathtext.default'], 'm',
state.fontsize, state.dpi)
width = metrics.advance
self._em_width_cache[key] = width
return Kern(width * percentage)

Making the following change does not break any Mathtext tests. (On my system. Though 120 tests get skipped, so I can't be sure.)

diff --git a/lib/matplotlib/_mathtext.py b/lib/matplotlib/_mathtext.py
index 3c4be21445..c630249282 100644
--- a/lib/matplotlib/_mathtext.py
+++ b/lib/matplotlib/_mathtext.py
@@ -1941,7 +1941,7 @@ class Parser:
         width = self._em_width_cache.get(key)
         if width is None:
             metrics = state.font_output.get_metrics(
-                state.font, mpl.rcParams['mathtext.default'], 'm',
+                'it', mpl.rcParams['mathtext.default'], 'm',
                 state.fontsize, state.dpi)
             width = metrics.advance
             self._em_width_cache[key] = width

Is it worth fixing?

Operating system

Linux Mint 20.2 Uma

Matplotlib Version

3.5.0.dev5379+g37ccdca502

Matplotlib Backend

GTK3Agg

Python version

3.8.10

Jupyter version

6.0.3

Installation

git checkout

@anntzer
Copy link
Contributor

anntzer commented Jul 22, 2022

Actually that's not really what 1em is (https://tex.stackexchange.com/questions/98127/where-do-the-values-of-1em-and-1ex-come-from), but using that definition basically breaks all baseline images, whereas your proposed change seems the practical way out for now (using 'rm' instead of 'it' also breaks all baseline images). So this seems OK for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants