Note that the underline is drawn below the lowest descender. This is consistent with TeX. (The PNG images suffer from what I assume is pixel rounding.)

Fixes#14235.
Closes#15624

PR Checklist

Tests and Styling

Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

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

Support\underline in Mathtext.

91a8d98

tfpf marked this pull request as ready for review

August 14, 2022 13:06

oscargus added the topic: text/mathtext label

Aug 14, 2022

Copy link

Member

oscargus commentedAug 14, 2022

Looks good! I think it makes sense to add a brief What's new for this to make users aware that it is added.

Also, I think it may be enough to have a "lightweight" test for this as it doesn't seem very dependent on the font:

matplotlib/lib/matplotlib/tests/test_mathtext.py

Lines 124 to 132 inecba9d2

	# 'lightweight' tests test only a single fontset (dejavusans, which is the
	# default) and only png outputs, in order to minimize the size of baseline
	# images.
	lightweight_math_tests= [
	r'$\sqrt[ab]{123}$',# github issue #8665
	r'$x \overset{f}{\rightarrow} \overset{f}{x} \underset{xx}{ff} \overset{xx}{ff} \underset{f}{x} \underset{f}{\leftarrow} x$',# github issue #18241
	r'$\sum x\quad\sum^nx\quad\sum_nx\quad\sum_n^nx\quad\prod x\quad\prod^nx\quad\prod_nx\quad\prod_n^nx$',# GitHub issue 18085
	r'$1.$ $2.$ $19680801.$ $a.$ $b.$ $mpl.$',
	]

tfpf added2 commits

August 15, 2022 10:11

Test for\underline in Mathtext.

4bb5284

"What's New" entry for Mathtext\underline.

b87c1b4

tfpf force-pushed theunderline branch from771db39 tob87c1b4Compare

August 15, 2022 05:06

oscargus approved these changes

Aug 15, 2022

View reviewed changes

timhoffm requested a review fromanntzer

August 15, 2022 08:04

Copy link

Member

timhoffm commentedAug 15, 2022

@anntzer I think you as our TeX expert should review this.

Copy link

Contributor

anntzer commentedAug 15, 2022

Sure, l'll need a couple of days though.

Copy link

Contributor

This seems reasonable, but 1) do you know why the overline() implementation is so different, using a fill instead? (which seems inconsistent with the TeXbook); 2) the TeXbook (rules G9/G10 p443) add an extrathickness of whitespace clearance over the overline / under the underline; do you know why they are not needed here (if they indeed aren't)?

Copy link

ContributorAuthor

tfpf commentedAug 20, 2022•
edited
Loading

the TeXbook (rules G9/G10 p443) add an extra thickness of whitespace clearance over the overline / under the underline

I think that extra space is used to set the minimum leading.

If the leading is high, the vertical distance between the tops of the Ts and the underscores on the previous lines remains unchanged even if the underscores are underlined. (As if the underline wasn't there at all.)

Upon reducing the leading, the Ts remain some distance away from the underline. It's like the underline forces the next line to go lower if it is too close.

I have not looked through Mathtext's leading rules, but it certainly looks like it's better to follow what TeX does.~~I will update this.~~ Getting second thoughts about this now. Should I add that space at the bottom, or leave this as it is?