Fix spacing in r"$\max f$".#30715
Conversation
timhoffm
left a comment
There was a problem hiding this comment.
LGTM, still some tests are failing.
lib/matplotlib/_mathtext.py
Outdated
| result = Hlist([ | ||
| vlt, | ||
| *([self._make_space(self._space_widths[r'\,'])] | ||
| if self._needs_space_after_subsuper else []), | ||
| ]) | ||
| self._needs_space_after_subsuper = False | ||
| return [result] |
There was a problem hiding this comment.
Optional alternative spelling. Choose whichever you like.
| result = Hlist([ | |
| vlt, | |
| *([self._make_space(self._space_widths[r'\,'])] | |
| if self._needs_space_after_subsuper else []), | |
| ]) | |
| self._needs_space_after_subsuper = False | |
| return [result] | |
| spacing_nodes = ( | |
| [self._make_space(self._space_widths[r'\,'])] | |
| if self._needs_space_after_subsuper else [] | |
| ) | |
| self._needs_space_after_subsuper = False | |
| return [Hlist([vlt, *spacing_nodes])] |
|
Testing for the space is already provided by mathtext1_dejavusans_06 (after the |
timhoffm
left a comment
There was a problem hiding this comment.
I'll leave this open, so that you can decide yourself on which branch this should go.
|
Mostly that's a decision for @QuLogic, I'd say. |
Previously, in a mathtext string like `r"$\sin x$"`, a thin space would
(correctly) be added between "sin" and "x", but that space would be
missing in expressions like `r"$\max f$"`. The difference arose because
of the slightly different handling of subscripts and superscripts
after the `\sin` and `\max` operators: `\sin^n` puts the superscript as
a normal exponent, but `\max_x` puts the subscript centered below the
operator name ("overunder symbol). The previous code for inserting the
thin space did not handle the "overunder" case; fix that. The new
behavior is tested by the change in test_operator_space, as well as by
mathtext1_dejavusans_06.
The change in mathtext_foo_29 arises because the extra thin space now
inserted after `\limsup` slightly shifts the centering of the whole
string. Ideally that thin space should be suppressed if there's no
token after the operator, but that's not something currently implemented
either for e.g. `\sin` (compare e.g. the right-alignments in
`text(.5, .9, r"$\sin$", ha="right"); text(.5, .8, r"$\mathrm{sin}$", ha="right"); axvline(.5)`
where the extra thin space after `\sin` is visible), so this patch just
makes things more consistent.
Rename _in_subscript_or_superscript to the more descriptive _needs_space_after_subsuper; simplify its setting in operatorname(); avoid the need to introduce an extra explicitly-typed spaced_nucleus variable.
|
There shouldn't be any reason why it can't go on the |
905fd51 to
17428e3
Compare
Fix spacing in r"$\max f$".
This includes images changes for the following pull requests / commits: * [Fix center of rotation with rotation_mode='anchor'](matplotlib#29199) (c44db77) * [Remove ttconv backwards-compatibility code](matplotlib#30145) (8caff88) * [Remove kerning_factor from tests](matplotlib#29816) (7b4d725) * [Set text hinting to defaults](matplotlib#29816) (8255ae2) * [Update FreeType to 2.13.3](matplotlib#29816) (89c054d) * [Implement text shaping with libraqm](matplotlib#30000) (b0ded3a, 9813523) * [Add language parameter to Text objects](matplotlib#29794) (7ce8eae) * [Fix auto-sized glyphs with BaKoMa fonts](matplotlib#29936) (3ba2c13) * [pdf: Improve text with characters outside embedded font limits](matplotlib#30512) (b70fb88, 6cedcf7) * [Prepare `CharacterTracker` for advanced font features](matplotlib#30608) (8274e17, 70dc388, df670cf, ed5e074) * [Add font feature API to Text](matplotlib#29695) (972a688) * [Fix spacing in r"$\max f$"](matplotlib#30715) (4a99a83) * [Implement libraqm for vector outputs](matplotlib#30607) (bd17cd4) * [Drop the FT2Font intermediate buffer](matplotlib#30059) (9d7d7b4) * [Rasterize dvi files without dvipng](matplotlib#30039) (7627118) * [Update bundled FreeType and HarfBuzz libraries](matplotlib#30938) (a161658, 9619bcc) * [Fix positioning of wide mathtext accents](matplotlib#31069) (c2fa7ba) * [Refactor RendererAgg.draw_{mathtext,text,tex} to use same base algorithm](matplotlib#31085) (931bcf3) * [Implement TeX's fraction and script alignment](matplotlib#31046) (94ff452, 4bfa0f9, 1cd8510) * [Fix confusion between text height and ascent in metrics calculations](matplotlib#31107) (60f2310) * [mathtext: Fetch quad width & axis height from font metrics](matplotlib#31110) (692df3f, 383028b) * [mathtext: add mathnormal and distinguish between normal and italic family](matplotlib#31121) (a6913f3) * [ENH: Ignore empty text for tightbbox](matplotlib#31285) (d772043) * [Drop axis_artist tickdir image compat, due to text-overhaul merge](matplotlib#31281) (2057583) * [text: Use font metrics to determine line heights](matplotlib#31291) (3ab6a27, d961462, 97f4943) * [ps/pdf: Override font height metrics to support AFM files](matplotlib#31371) (e0913d4) * [TST: Cleanup back-compat code in tests touched by text overhaul](matplotlib#31295) (7c33379) * [TST: Set tests touched by text overhaul to mpl20 style](matplotlib#31300) (41c4d8d)
First commit (main implementation):
Previously, in a mathtext string like
r"$\sin x$", a thin space would(correctly) be added between "sin" and "x", but that space would be
missing in expressions like
r"$\max f$". The difference arose becauseof the slightly different handling of subscripts and superscripts
after the
\sinand\maxoperators:\sin^nputs the superscript asa normal exponent, but
\max_xputs the subscript centered below theoperator name ("overunder symbol). The previous code for inserting the
thin space did not handle the "overunder" case; fix that. The new
behavior is tested by the change in test_operator_space, as well as by
mathtext1_dejavusans_06.
The change in mathtext_foo_29 arises because the extra thin space now
inserted after
\limsupslightly shifts the centering of the wholestring. Ideally that thin space should be suppressed if there's no
token after the operator, but that's not something currently implemented
either for e.g.
\sin(compare e.g. the right-alignments intext(.5, .9, r"$\sin$", ha="right"); text(.5, .8, r"$\mathrm{sin}$", ha="right"); axvline(.5)where the extra thin space after
\sinis visible), so this patch justmakes things more consistent.
Second commit (cleanup):
Rename _in_subscript_or_superscript to the more descriptive
_needs_space_after_subsuper; simplify its setting in operatorname();
avoid the need to introduce an extra explicitly-typed spaced_nucleus
variable.
(See previous work in #17890 and #23243.)
@QuLogic This PR also changes a few baseline images so maybe it could get folded into the text-overhaul branch (for now I labeled it as such), but the changes are limited and standalone so it could also go in as a normal PR if that makes things simpler for you, I don't mind either way.
PR summary
PR checklist