Support units for the third `\genfrac` argument #22171

oscargus · 2022-01-09T13:35:34Z

PR Summary

Closes #19001.

This also means that it is rather easy to support units for other things.

One doubt though: I couldn't figure out the standard unit in mathtext. Right now, I have assumed pt, but I suspect it may not be correct. I'll update with a test once I know the answer... However, as I have local test errors on image comparisons (for pdf), I am not convinced that the tests will pass. These are the ones I currently get:

FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavusans-72] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 2.524):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavusans-77] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 2.524):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-20] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 1.785):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-33] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 1.785):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-36] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 1.785):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-37] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 1.785):
FAILED test_mathtext.py::test_mathtext_rendering[pdf-mathtext-dejavuserif-75] - matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 2.524):

Is there a "better" way to generate test images that are more likely to pass on the CI?

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
[N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

anntzer · 2022-01-13T22:42:09Z

lib/matplotlib/_mathtext.py

+        'cm': 7227/254,
+        'in': 7227/100,
+        'ex': 35271/8192,
+        'em': 655361/65536,


ex and em are probably not fixed values as they should(?) depend on the actual font being used, e.g. whether we're in a mathbf or a mathcal or whatnot. OTOH we don't necessarily need to support them for now?

anntzer · 2022-01-13T22:43:30Z

lib/matplotlib/_mathtext.py

+        'in': 7227/100,
+        'ex': 35271/8192,
+        'em': 655361/65536,
+        'mu': 7227/2540000,


I can't find a ref for mu, nc, nd as standard tex units, perhaps you can link their definition? (as a comment here at least, not in the code)

I used some stack exchange thread combined with other sources, but cannot seem to find where I got ex and em from now. I agree with that they shouldn't be there though (just added what was there without really thinking about the meaning).

Here are some better sources:
https://en.wikibooks.org/wiki/LaTeX/Lengths
https://latexref.xyz/Units-of-length.html

(And here is a good discussion why ex and em are bad choices: https://tex.stackexchange.com/questions/4239/which-measurement-units-should-one-use-in-latex
)

And here is for mu: https://sv.overleaf.com/learn/latex/Lengths_in_LaTeX

anntzer · 2022-01-13T22:49:18Z

I agree that the "reference unit" should be clarified, it's not clear to me either. Also note that TeX basically operates in a world of 72.27dpi (aka 72 "big points per inch") exactly whereas we don't, so I'm not sure if we can have correct physical dimensions (in, cm, mm, etc., e.g. a height of 1cm should occupy half of a figure that is 2cm tall -- by the way this would be a good example to post in this thread to show that the sizes are indeed correct) and correct point-based dimensions (e.g. a height of 12pt should occupy whatever that means wrt. text of font size 12). (I'm not sure I have a good answer, but probably the design choices will need to be made explicit.)

jklymak · 2022-01-17T12:19:01Z

Ooops sorry marked ready for review by accident

WRT em and ex I think they are perfectly reasonable units, particularly for the fraction line which you may think would thicken or thin relative to the font size.

oscargus · 2022-01-17T13:04:44Z

WRT em and ex

Yes, they are reasonable as such. The problem is that they are not static, so one will then need to figure out for each font what the appropriate scale factor is. Unless I find a "simple" solution for that (maybe @anntzer knows how one can query this), I do not think it will be included in this PR.

Reading up a bit more on https://tex.stackexchange.com/questions/4239/which-measurement-units-should-one-use-in-latex it says

 Bringhurst says: (2.1.1) "1 em is a distance equal to the type size", so you can be more certain than "usually consider".

This would make it a bit easier at least by using the font size as the em scale factor.

For ex one may have to measure though.

oscargus · 2022-01-30T10:50:23Z

I have removed ex and added proper support for em (based on pt being used and the statement "1 em is a distance equal to the type size".

Now I have problem generating \genfrac with usetex=True for comparison, but I seem to recall that the results were matching pretty well. Will try to solve that and provide some examples.

oscargus · 2022-01-30T11:03:37Z

Forgot about amsmath...

Here are some examples:

Rendered with mathtext and latex respectively. As far as I can tell, the current scaling matches the LaTeX rendering if nothing else. The size of the bar is stated in numerator and denominator.

Then, the bug in #22172 is manifested. I have a small hack for that leading to:

Not sure if I should include that? It seems like the approach in #20627 is fundamentally better (since it actually considers how LaTeX does it, not just the result of playing around), but that currently doesn't seem to handle wide bars (which may not really be a problem).

tfpf · 2022-05-08T14:49:43Z

#20627 has been replaced by #22852, which does it the same way LaTeX does (i.e. with font constants and with the thickness of the line taken into account).

anntzer · 2022-05-21T12:01:58Z

lib/matplotlib/_mathtext.py

@@ -2004,6 +2006,10 @@ def __init__(self):

        p.float_literal <<= Regex(r"[-+]?([0-9]+\.?[0-9]*|\.[0-9]+)")
        p.int_literal   <<= Regex("[-+]?[0-9]+")
+        p.unit          <<= Regex("(pt|mm|cm|in|em|mu|"
+                                  "sp|bp|dd|pc|nc|nd|cc)")
+        p.float_with_wo_unit <<= Group((p.float_literal + p.unit) |


what's "wo"?

anntzer · 2022-05-21T12:03:04Z

lib/matplotlib/_mathtext.py

@@ -2004,6 +2006,10 @@ def __init__(self):

        p.float_literal <<= Regex(r"[-+]?([0-9]+\.?[0-9]*|\.[0-9]+)")
        p.int_literal   <<= Regex("[-+]?[0-9]+")
+        p.unit          <<= Regex("(pt|mm|cm|in|em|mu|"


I think you can get rid of the grouping parentheses, which should be marginally more efficient.

anntzer · 2022-05-21T12:18:26Z

If I understand correctly, this makes the old "unitless" genfrac use points (= pixels), and then defines units using tex's ratios (1in=72pt, etc.), which means that a 1in thickness will not actually be e.g. 1/3 of a 3in figure unless working at 72dpi: try e.g.

from pylab import *
rcParams["text.latex.preamble"] = r"\usepackage{amsmath}"
figure(figsize=(3, 3), dpi=100).text(.5, .5, r"$\genfrac{}{}{1in}{1}{a}{b}$", size=24, usetex=False)
figure(figsize=(3, 3), dpi=100).text(.5, .5, r"$\genfrac{}{}{1in}{1}{a}{b}$", size=24, usetex=True)

I'm not sure that's really what we want (basically #22171 (comment))? The parser does have dpi information available (get_state().dpi); perhaps it should scale physical units accordingly instead? (I'm not actually sure, but asking for a 1in thick rule and getting something else is a bit annoying?) In any case the behavior we decide on should be documented in the comments, at least.

This also clearly displays the #22172 bug (but I understand that #22852 is working on that) and there's also a vertical alignment problem (which also seems fixed bu #22852?).

oscargus · 2022-05-24T07:54:03Z

But that, somehow, creates a problem already now? I'm not saying that this is the way to go, but I am quite sure that it is not introducing any new issues? Except that maybe one do expect a certain result as one can state the units and the previous "unitless" one, there were really no expectations (except that possibly one would guess it was points).

But you are probably correct that ideally one should take the dpi into consideration.

anntzer · 2022-05-24T21:05:02Z

The new issue is that someone could write \genfrac...{1cm}... and not get what they expected (which I would argue is worse than erroring out -- at least in that case you know the feature is not supported).

oscargus · 2022-05-25T10:49:48Z

and not get what they expected

Fair point.

Assuming that I manage to solve that part, should we also change the default to mean points? (Which I still sort of guess that most people assume it means?)

anntzer · 2022-05-25T12:27:42Z

I don't really think you can change the default behavior here?

jklymak · 2022-06-23T08:00:44Z

@oscargus what is the status here? I've moved to draft, but feel free to move back. If @tfpf is good with this, I can review based on their review, and hopefully @anntzer can provide a second review.

oscargus · 2022-06-23T08:30:20Z

@oscargus what is the status here?

@anntzer made a fair point about dpi-scaling that I have not had time to look into, so moving to draft is a good call.

I think it should be quite straightforward to sort it out though, given time (and inspiration, but that is a bit random).

oscargus force-pushed the genfracunits branch from 96b4bae to 46d2a1e Compare January 9, 2022 13:41

This was referenced Jan 9, 2022

[Bug]: \genfrac has bad spacing with (high) custom ruler #22172

Open

Support \text in mathtext #22173

Merged

dstansby added the topic: text/mathtext label Jan 9, 2022

oscargus mentioned this pull request Jan 12, 2022

Cleanup _mathtext internal API #22204

Merged

6 tasks

anntzer reviewed Jan 13, 2022

View reviewed changes

QuLogic changed the title ~~mathtext now supports units for the third \genfrac argument~~ Support units for the third \genfrac argument Jan 14, 2022

jklymak marked this pull request as ready for review January 17, 2022 12:17

oscargus marked this pull request as draft January 17, 2022 13:08

mathtext now supports units for the third genfrac argument

b8b3d77

oscargus force-pushed the genfracunits branch from 46d2a1e to b8b3d77 Compare January 30, 2022 10:36

oscargus marked this pull request as ready for review January 30, 2022 11:04

oscargus added the status: needs review label Feb 19, 2022

oscargus added this to the v3.6.0 milestone Mar 25, 2022

oscargus mentioned this pull request May 8, 2022

[ENH]: Use \genfrac in display style? #23008

Closed

anntzer reviewed May 21, 2022

View reviewed changes

jklymak marked this pull request as draft June 23, 2022 08:00

oscargus removed the status: needs review label Jul 19, 2022

oscargus removed this from the v3.6.0 milestone Aug 19, 2022

github-actions bot added the status: needs rebase label Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support units for the third `\genfrac` argument #22171

Support units for the third `\genfrac` argument #22171

oscargus commented Jan 9, 2022 •

edited

Loading

anntzer Jan 13, 2022

anntzer Jan 13, 2022

oscargus Jan 17, 2022

oscargus Jan 17, 2022

anntzer commented Jan 13, 2022 •

edited

Loading

jklymak commented Jan 17, 2022

oscargus commented Jan 17, 2022

oscargus commented Jan 30, 2022

oscargus commented Jan 30, 2022 •

edited

Loading

tfpf commented May 8, 2022

anntzer May 21, 2022

anntzer May 21, 2022

anntzer commented May 21, 2022

oscargus commented May 24, 2022

anntzer commented May 24, 2022

oscargus commented May 25, 2022

anntzer commented May 25, 2022

jklymak commented Jun 23, 2022

oscargus commented Jun 23, 2022

Support units for the third \genfrac argument #22171

Are you sure you want to change the base?

Support units for the third \genfrac argument #22171

Conversation

oscargus commented Jan 9, 2022 • edited Loading

PR Summary

PR Checklist

anntzer Jan 13, 2022

Choose a reason for hiding this comment

anntzer Jan 13, 2022

Choose a reason for hiding this comment

oscargus Jan 17, 2022

Choose a reason for hiding this comment

oscargus Jan 17, 2022

Choose a reason for hiding this comment

anntzer commented Jan 13, 2022 • edited Loading

jklymak commented Jan 17, 2022

oscargus commented Jan 17, 2022

oscargus commented Jan 30, 2022

oscargus commented Jan 30, 2022 • edited Loading

tfpf commented May 8, 2022

anntzer May 21, 2022

Choose a reason for hiding this comment

anntzer May 21, 2022

Choose a reason for hiding this comment

anntzer commented May 21, 2022

oscargus commented May 24, 2022

anntzer commented May 24, 2022

oscargus commented May 25, 2022

anntzer commented May 25, 2022

jklymak commented Jun 23, 2022

oscargus commented Jun 23, 2022

Support units for the third `\genfrac` argument #22171

Support units for the third `\genfrac` argument #22171

oscargus commented Jan 9, 2022 •

edited

Loading

anntzer commented Jan 13, 2022 •

edited

Loading

oscargus commented Jan 30, 2022 •

edited

Loading