PGF: Consistently set LaTeX document font size #26893

Socob · 2023-09-23T07:50:25Z

PR summary

Currently, when using the PGF backend, different LaTeX document font size settings are used when measuring widths vs. producing output. Although this is usually not a problem (since all text produced by the PGF backend locally sets the font properties), packages used in the custom preamble (such as unicode-math) may depend on the font size settings, leading to incorrect width measurements since different outcomes are produced in the two cases. This fixes the issue by always setting the LaTeX font size to the rc setting font.size.

Closes #26892.

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
[N/A] Plotting related features are demonstrated in an example
[N/A] New Features and API Changes are noted with a directive and release note
[N/A] Documentation complies with general and docstring guidelines

This could cause a mismatch between lengths when measuring vs. producing output for things that depend on the font size setting at the beginning of the document, e.g. unicode-math. Use \documentclass{article} without options here, which defaults to a font size of 10pt (same as the matplotlib rc default for font.size).

This allows the output .pgf file to be used in external documents with arbitrary font size settings (which should match the rc setting font.size). This avoids mismatches between matplotlib and external LaTeX documents for things that depend on the font size setting (e.g. unicode-math). If the LaTeX package scrextend is present, this also adjusts the standard LaTeX font commands (\tiny, ..., \normalsize, ..., \Huge) relative to font.size.

github-actions

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Socob · 2023-09-23T08:05:41Z

Not sure what’s going on with the flake8 test thing – it doesn’t look like I changed anything about blank lines where it’s complaining.

lib/matplotlib/backends/backend_pgf.py

oscargus

I think this looks good!

However, it may be worthwhile adding an image test to confirm that the new behavior is kept.

Also, it may be that one will need to install scrextend in the CI to get it going (not sure if it is included in any standard distribution or not).

Finally, it may be helpful to add a note that this will work better if scrextend is available. Maybe as an additional bullet at the end of https://github.com/matplotlib/matplotlib/blob/main/galleries/users_explain/text/pgf.py
(https://matplotlib.org/stable/users/explain/text/pgf.html)

Socob · 2023-10-03T16:02:02Z

However, it may be worthwhile adding an image test to confirm that the new behavior is kept.

Sounds like a good idea – I’ll get on that.

Also, it may be that one will need to install scrextend in the CI to get it going (not sure if it is included in any standard distribution or not).

scrextend is part of the KOMA Script bundle, which is very widely used. In TeX Live, it’s part of collection-latexrecommended, the same as fontspec and underscore which are currently already loaded unconditionally. So normally, it should be present if the other two are.

Either way, the use of scrextend is really just to set up the font size commands \tiny, ..., \normalsize, ..., \Huge with sensible values. I don’t think these are commonly used, and the fallback takes care of the one that really matters when it comes to this issue (\normalsize).

Finally, it may be helpful to add a note that this will work better if scrextend is available. Maybe as an additional bullet at the end of https://github.com/matplotlib/matplotlib/blob/main/galleries/users_explain/text/pgf.py (https://matplotlib.org/stable/users/explain/text/pgf.html)

Not sure if this is necessary given the use of fontspec and underscore (see above)? The only reason I added the \IfFileExists check is really just because scrextend is not absolutely necessary to fix the main issue, and I wanted to assume as little as possible about the user’s TeX installation.

oscargus · 2023-10-04T11:42:43Z

Then it should be fine I think!

(Or rather, it would be nice that the required packages are explicitly stated somewhere, I may have missed it though, but this PR doesn't strictly add something to that.)

tacaswell · 2023-12-20T22:06:13Z

@pwuertz Do you have bandwidth to review this?

lib/matplotlib/backends/backend_pgf.py

Socob · 2024-04-22T21:17:48Z

I’ve made the last requested change and rebased. So presumably it’s ready to merge?

lib/matplotlib/backends/backend_pgf.py

voidstarstar · 2024-04-23T14:28:44Z

I think that the documentclass is also set in texmanager.py for use with the usetex flag. In the future, should _DOCUMENTCLASS cover this as well?

matplotlib/lib/matplotlib/texmanager.py

Line 203 in eb62d69

r"\documentclass{article}",

Socob · 2024-04-23T15:57:19Z

Since texmanager.py is used for a completely different purpose, I didn’t touch it. I also don’t think that texmanager.py and PGF should necessarily use the same \documentclass.

Socob · 2024-04-23T17:30:59Z

Once again added all requested changes!

Socob · 2024-04-29T19:02:13Z

Is there anything else that needs to be done? I don’t know what the codecov test is complaining about, nothing in there looks related to my changes.

anntzer · 2024-05-01T21:58:51Z

lib/matplotlib/backends/backend_pgf.py

        *([
+            r"\ifdefined\pdftexversion\else  % non-pdftex case.",


Is there a reason for the change here and immediately below?

That’s 14a5900. I guess it’s only tangentially related to the rest of this PR, but I noticed that \usepackage{fontspec} was always included in the preamble, which is not necessarily desirable when doing custom font setup (i. e. pgf.rcfonts=False).

I think the ability to exclude \usepackage{fontspec} is a very good improvement. It's sometimes necessary for consistency. From the documentation it sounds like fontspec should only be used when pgf.rcfonts=True, so this looks like an additional bugfix to me.

anntzer · 2024-05-01T21:58:57Z

lib/matplotlib/backends/backend_pgf.py

            r"  \%s{%s}[Path=\detokenize{%s/}]"
            % (command, path.name, path.parent.as_posix())
            for command, path in zip(
                ["setmainfont", "setsansfont", "setmonofont"],
                [pathlib.Path(fm.findfont(family))
                 for family in ["serif", "sans\\-serif", "monospace"]]
            )
-        ] if mpl.rcParams["pgf.rcfonts"] else []),
-        r"\fi",
+        ] + [r"\fi"] if mpl.rcParams["pgf.rcfonts"] else []),


... i.e. here?

anntzer · 2024-05-01T22:00:40Z

lib/matplotlib/backends/backend_pgf.py

@@ -185,7 +205,7 @@ class LatexManager:
    @staticmethod
    def _build_latex_header():
        latex_header = [
-            r"\documentclass{article}",


I think these could have been left as they were originally (but removing the 12pt below) no? especially considering that we may want to make the documentclass configurable in the future, which would be more awkward to do if referencing a global.

Not sure what you mean? These all have to be identical, so introducing a single name for them is just DRY. And in fact one of the first things one would do when making the documentclass configurable. I don’t see how it matters whether it’s a global constant or something else – that can always be changed with no effort. The point is to use a single variable for things that have to be the same.

anntzer · 2024-05-01T22:03:32Z

lib/matplotlib/tests/test_backend_pgf.py

+)
+@pytest.mark.backend('pgf')
+@image_comparison(['pgf_document_font_size.pdf'], style='default', remove_text=True)
+def test_document_font_size():


do you really need unicode-math for this test? I'd say it's not particularly relevant, no?

you could do without the plot at all and simply use figtext() to draw text on an empty figure.

Not unicode-math specifically, no. But if I remember correctly, unicode-math (and other math font packages) select the font variant they use at package load time based on the document font size, and these variants have different metrics even for the same font size, which is what caused the original issue. So I’d either need unicode-math or some random font package with the same behavior, and unicode-math seems much more universal (especially since matplotlib already uses it by default).

I suppose I could. I just took the easiest path and used the example I already had in [Bug]: PGF font size mismatch between measurement and output #26892. Let me know if you really want me to make and test a new example.

Actually, I think I was mistaken with my comment that matplotlib uses unicode-math (I was thinking of fontspec). Nevertheless, the rest still applies.

voidstarstar · 2024-05-02T18:16:32Z

I think this PR tries to fix too much. Isn't fixing the 12pt mismatch the only thing necessary to fix the original bug demonstrated by the code for reproduction? Everything else seems to target a slightly different bug (or bugs) that I do not know how to reproduce, but they sound to me like they might be solved by setting a custom documentclass (see #28119).

Maybe I'm wrong, but I thought that the font.size rcParam option was supposed to serve a different purpose than the global document text size and I'm now wondering if this PR confuses the two. I thought that font.size was the default size for the figure and that matplotlib provides special sizes relative to font.size for this purpose already. Instead of basing the LaTeX font size commands (\tiny, etc.) on font.size, wouldn't a better solution be to just allow the user to set their own custom documentclass (which can include their own global document text size)? I think this would fix the implicit dependence on the size problem mentioned in the original issue.

In other words, if the user could set the documentclass in matplotlib to match their own custom documentclass, wouldn't that fix this issue in a better way without needing to force the global document font size to match font.size?

Socob · 2024-05-02T19:34:30Z

I really didn’t expect “tries to fix too much” on a PR that adds/changes 21 lines of code (excluding comments/tests/blanks)! I don’t know what you mean by “everything else” beyond adjusting the LaTeX font commands?

Since there is currently no way to set the documentclass, font.size is the only available information on what the user’s document font size is. The matplotlib internal sizes are probably preferable in some cases (when they can be used), but since I was setting \normalsize, I thought it’d be more consistent to adjust the other LaTeX font size commands accordingly (this is 3 lines of code in the PR). Since there is no documentation on this, users will expect that these font commands will work “as expected”, as evidenced by posts such as this one.

Sure, making the documentclass configurable may be a better way to fix this than my (minimal!) change. Feel free to write that PR!

Socob · 2024-05-03T14:14:49Z

@anntzer Do my answers resolve your review comments or do you want me to change anything about the PR?

voidstarstar · 2024-05-03T17:55:29Z

I really didn’t expect “tries to fix too much” on a PR that adds/changes 21 lines of code (excluding comments/tests/blanks)! I don’t know what you mean by “everything else” beyond adjusting the LaTeX font commands?

Yes, the "everything else" are the other font related changes (fixing a scope bug by defaulting to \familydefault, fixing fontspec to depend on pgf.rcfonts, and using scrextend to set relative text sizes). To correct my previous post, these would not be fixed by #28119.

To be clear, I wasn't saying that the changes are bad or that they should not be merged. I actually think most are strong improvements and a dependency for a PR I'm currently writing too. I just did not fully understand the motivation behind the scrextend related code because I think in some cases it might introduce a consistency issue since it is a global setting. Thank you for the SO link, it helps me better understand the use case.

voidstarstar · 2024-05-04T04:29:34Z

The following is a proof of concept of the possible consistency issue I mentioned:

LaTeX source:

\documentclass[12pt]{article}
\usepackage{pgf}
\begin{document}
\Huge Before
\input{plot.pgf}
\Huge After
\end{document}

Python source:

import matplotlib
matplotlib.use("pgf")
import matplotlib.pyplot as plt

# Set up PGF
matplotlib.rcParams.update({
    "pgf.texsystem": "lualatex",
    "font.family": "serif",
    "text.usetex": True,
    "pgf.rcfonts": False,
    "font.size": 1,
    "figure.autolayout": True,
    "axes.labelsize": "xx-small",
    })

# Create some sample data
x = [0, 1, 2, 3, 4]
y = [0, 0.2, 0.4, 0.1, 0.3]

# Create the plot
fig, ax = plt.subplots(figsize=(4.5, 3))
ax.plot(x, y)

ax.set_title("Sample Plot")

ax.set_xlabel(r"\Huge X")

ax.set_ylabel("Y")

# Save the plot
plt.savefig("plot.pgf")
plt.savefig("plot.png", dpi=300)

I think the above example might demonstrate the problem I was alluding to. I use extremely big/small sizes to make the problem more obvious visually. The font.size is set to 1. This causes scrextend to change \Huge to be much smaller (since it's relative to 1 instead of 12). Matplotlib then internally renders the xlabel r"\Huge X" which is rendered small as expected. The problem arises when the pgf file is imported into LaTeX. The LaTeX document preamble does not include scrextend, so the xlabel is much bigger than expected and therefore is misaligned. We can't include scrextend in the preamble because it changes the font globally, which will affect the other text in the document.

My recommended solution would be to leave the \normalsize and other commands (\tiny, etc.) alone to match the LaTeX file, or if you must change them using scrextend, do so using pgf.preamble. Ideally, I think the plot should not rely on changing any global settings.

Sure, making the documentclass configurable may be a better way to fix this than my (minimal!) change. Feel free to write that PR!

I'm currently working on #28167 for this.

anntzer · 2024-05-04T11:00:13Z

Thank you @voidstarstar for your example. I agree we can't assume that the user will indeed be using koma-script just because it happens to be present in the install (if I understood the problem you're mentioning correctly).

Socob · 2024-05-04T12:54:23Z

I think there is a lot of confusion going on here. First of all, \usepackage[fontsize=<N>pt]{scrextend} has nothing to do with whether or not the user is using KOMA-Script. It simply replicates what would happen when setting the document font size, as in \documentclass[<N>pt]{article}. Now, why did I not simply pass the font size to the documentclass? Because the standard LaTeX classes, like article, don’t support arbitrary font sizes, wheres KOMA-Script does. Since the user cannot set the documentclass, this is the next-best thing to support as many cases as possible. (And for obvious reasons, I didn’t want to change the documentclass used by the PGF backend in this PR.)

Now, for @voidstarstar’s example: Of course, the assumption is that the user will set font.size to match the font size of their document! What scenario or use case would there be where any other choice makes sense?

My recommended solution would be to leave the \normalsize and other commands (\tiny, etc.) alone to match the LaTeX file

How? matplotlib does not know anything about the user’s LaTeX file! That’s the whole point: Aligning the document font size used by the PGF backend with font.size allows the user to control what font size is used internally by matplotlib, and to set this to match their own document! If we don’t do this, we have to arbitrarily hard-code a fixed document font size, making it impossible to include the generated PGF in LaTeX documents with other font sizes (there are workarounds of course, see #26892).

Now, I agree that the ideal solution for this problem would be for the user to be able to set the documentclass, at which point the code I’ve added which is setting the document font size could be removed. But I just wanted to fix the immediate problem to make things work by default, without workarounds in the custom preamble when making this PR!

voidstarstar · 2024-05-04T15:55:04Z

Now, for @voidstarstar’s example: Of course, the assumption is that the user will set font.size to match the font size of their document! What scenario or use case would there be where any other choice makes sense?

This seems like the core of my confusion. Please consider the following use case:

A user is writing a LaTeX paper where the plots are somewhat small for space reasons due to journal imposed page limitations. The font size in the plots must be smaller than the document font size in order for the text to fit. By default, all of the text element sizes (axes.titlesize, axes.labelsize, xtick.labelsize, legend.fontsize, etc.) use relative sizing (large, medium, etc.). This means the user can change font.size and the entire figure will scale proportionally. These relative sizes are converted to absolute pt sizes when writing the pgf file, so nothing is dependent upon the default document font size.

My recommended solution would be to leave the \normalsize and other commands (\tiny, etc.) alone to match the LaTeX file

How? matplotlib does not know anything about the user’s LaTeX file! That’s the whole point: Aligning the document font size used by the PGF backend with font.size allows the user to control what font size is used internally by matplotlib, and to set this to match their own document! If we don’t do this, we have to arbitrarily hard-code a fixed document font size, making it impossible to include the generated PGF in LaTeX documents with other font sizes (there are workarounds of course, see #26892).

The 2nd half of my sentence mentions using pgf.preamble which is what you did in your workaround. The issue I have with making the workaround built in to matplotlib is that it breaks the use cases where font.size does not match the document size.

Now, I agree that the ideal solution for this problem would be for the user to be able to set the documentclass, at which point the code I’ve added which is setting the document font size could be removed. But I just wanted to fix the immediate problem to make things work by default, without workarounds in the custom preamble when making this PR!

I agree. I'm personally not opposed to this as a temporary stepping stone solution.

Socob · 2024-05-04T23:25:47Z

A user is writing a LaTeX paper where the plots are somewhat small for space reasons due to journal imposed page limitations. The font size in the plots must be smaller than the document font size in order for the text to fit. By default, all of the text element sizes (axes.titlesize, axes.labelsize, xtick.labelsize, legend.fontsize, etc.) use relative sizing (large, medium, etc.). This means the user can change font.size and the entire figure will scale proportionally. These relative sizes are converted to absolute pt sizes when writing the pgf file, so nothing is dependent upon the default document font size.

Setting the document font size doesn’t impair this use case at all. You can still set font.size to something other than the final LaTeX document’s font size; there is only a problem if you’re doing something that’s inherently tied to the document font size, such as using LaTeX font size commands in the plot or using a package like unicode-math. Clearly, by using these, you’re necessarily introducing a dependence on the document font size into your plot. Only in this case, you have to set font.size to match your document font size. And in this scenario, I do have a hard time imagining how any other setting could make sense.

Let’s also keep in mind that you’d always have this problem with a hard-coded document font size (unless your document’s font size just happened to match the matplotlib-internal one), so I don’t see any alternative!

voidstarstar · 2024-05-05T01:32:07Z

Let’s also keep in mind that you’d always have this problem with a hard-coded document font size (unless your document’s font size just happened to match the matplotlib-internal one), so I don’t see any alternative!

That's a very good point, I agree. The hard-coded document font size is really the core issue, not your code. Your code at least has the benefit of allowing the user to set font.size to have some control over it. Until the hard-coded documentclass is removed, there will always be some use case that will be broken and the one you are targeting would also benefit more users.

Sorry about the confusion, you are correct. I retract my example/problem mentioned above. This PR has my 👍 (for whatever that's worth).

So it sounds to me like the best course of action would be:

First, PGF: Consistently set LaTeX document font size #26893 (this PR) gets merged.
Then, the temporary scrextend code introduced by this PR should be removed in Allow custom documentclass for pgf backend #28167 and replaced with a fully customizable documentclass.
Finally, Allow custom documentclass for pgf backend #28167 gets merged.

voidstarstar · 2024-05-07T12:12:57Z

Thank you @voidstarstar for your example. I agree we can't assume that the user will indeed be using koma-script just because it happens to be present in the install (if I understood the problem you're mentioning correctly).

@anntzer The problem I was mentioning was not really an issue with this PR, but rather an consequence of having a hard-coded documentclass. As @Socob mentioned (and I agree with), the code in this PR does not assume koma-script is being used. The command for conveniently changing the default sizes just happens to be part of that library, but seems to be often used on its own. Please disregard my previous objection and example code. My opinion is now LGTM!

anntzer

OK, this seems reasonable in any case and I'll trust @voidstarstar on the details.

Socob added 3 commits September 23, 2023 01:28

PGF: Only force loading fontspec if pgf.rcfonts=True

14a5900

Socob force-pushed the fix-pgf-fontsize branch from f44801a to d8ce139 Compare September 23, 2023 07:53

github-actions bot reviewed Sep 23, 2023

View reviewed changes

oscargus reviewed Sep 23, 2023

View reviewed changes

lib/matplotlib/backends/backend_pgf.py Show resolved Hide resolved

PGF: Remove unnecessary _usepackage_if_not_loaded

98673a0

oscargus added the backend: pgf label Oct 3, 2023

oscargus approved these changes Oct 3, 2023

View reviewed changes

PGF: Do not use \setmainfont if pgf.rcfonts=False

896b280

Socob force-pushed the fix-pgf-fontsize branch 3 times, most recently from 3a4a86e to ab50f7e Compare October 3, 2023 19:44

Add image test for matplotlib#26892

4a4a41f

Socob force-pushed the fix-pgf-fontsize branch from ab50f7e to 4736de1 Compare October 3, 2023 19:49

timhoffm reviewed Dec 21, 2023

View reviewed changes

lib/matplotlib/backends/backend_pgf.py Outdated Show resolved Hide resolved

This was referenced Apr 22, 2024

[Bug]: PGF backend uses incorrect xtick.labelsize and ytick.labelsize when they are small #28112

Closed

[ENH]: Allow a custom documentclass when using the pgf backend. #28119

Open

Prepend underscore to variable name DOCUMENTCLASS

0fec213

Socob force-pushed the fix-pgf-fontsize branch 2 times, most recently from 010af0d to 0fec213 Compare April 22, 2024 19:20

anntzer reviewed Apr 23, 2024

View reviewed changes

lib/matplotlib/backends/backend_pgf.py Show resolved Hide resolved

voidstarstar reviewed Apr 23, 2024

View reviewed changes

lib/matplotlib/backends/backend_pgf.py Outdated Show resolved Hide resolved

Socob added 2 commits April 23, 2024 11:59

PGF: Use \familydefault instead of \rmfamily if pgf.rcfonts=False

6d49b51

PGF: Expand comment on use of scrextend LaTeX package

f305f5f

anntzer reviewed May 1, 2024

View reviewed changes

voidstarstar mentioned this pull request May 4, 2024

Allow custom documentclass for pgf backend #28167

Open

5 tasks

anntzer approved these changes May 7, 2024

View reviewed changes

anntzer merged commit bc13abf into matplotlib:main May 7, 2024
43 of 44 checks passed

anntzer added this to the v3.10.0 milestone May 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PGF: Consistently set LaTeX document font size #26893

PGF: Consistently set LaTeX document font size #26893

Socob commented Sep 23, 2023

github-actions bot left a comment

Socob commented Sep 23, 2023

oscargus left a comment

Socob commented Oct 3, 2023 •

edited

oscargus commented Oct 4, 2023

tacaswell commented Dec 20, 2023

Socob commented Apr 22, 2024

voidstarstar commented Apr 23, 2024

Socob commented Apr 23, 2024

Socob commented Apr 23, 2024

Socob commented Apr 29, 2024

anntzer May 1, 2024

Socob May 2, 2024 •

edited

voidstarstar May 3, 2024

anntzer May 1, 2024

Socob May 2, 2024

anntzer May 1, 2024 •

edited

Socob May 2, 2024

anntzer May 1, 2024

Socob May 2, 2024

Socob May 3, 2024

voidstarstar commented May 2, 2024

Socob commented May 2, 2024

Socob commented May 3, 2024

voidstarstar commented May 3, 2024 •

edited

voidstarstar commented May 4, 2024 •

edited

anntzer commented May 4, 2024

Socob commented May 4, 2024

voidstarstar commented May 4, 2024

Socob commented May 4, 2024

voidstarstar commented May 5, 2024

voidstarstar commented May 7, 2024

anntzer left a comment

PGF: Consistently set LaTeX document font size #26893

PGF: Consistently set LaTeX document font size #26893

Conversation

Socob commented Sep 23, 2023

PR summary

PR checklist

github-actions bot left a comment

Choose a reason for hiding this comment

Socob commented Sep 23, 2023

oscargus left a comment

Choose a reason for hiding this comment

Socob commented Oct 3, 2023 • edited

oscargus commented Oct 4, 2023

tacaswell commented Dec 20, 2023

Socob commented Apr 22, 2024

voidstarstar commented Apr 23, 2024

Socob commented Apr 23, 2024

Socob commented Apr 23, 2024

Socob commented Apr 29, 2024

Choose a reason for hiding this comment

Socob May 2, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anntzer May 1, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voidstarstar commented May 2, 2024

Socob commented May 2, 2024

Socob commented May 3, 2024

voidstarstar commented May 3, 2024 • edited

voidstarstar commented May 4, 2024 • edited

anntzer commented May 4, 2024

Socob commented May 4, 2024

voidstarstar commented May 4, 2024

Socob commented May 4, 2024

voidstarstar commented May 5, 2024

voidstarstar commented May 7, 2024

anntzer left a comment

Choose a reason for hiding this comment

Socob commented Oct 3, 2023 •

edited

Socob May 2, 2024 •

edited

anntzer May 1, 2024 •

edited

voidstarstar commented May 3, 2024 •

edited

voidstarstar commented May 4, 2024 •

edited