Fix spacing in r"$\max f$". #30715
Merged
Fix spacing in r"$\max f$". #30715
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
timhoffm
reviewed
Nov 2, 2025
Member
timhoffm
left a comment
timhoffm
left a comment
There was a problem hiding this comment.
LGTM, still some tests are failing.
lib/matplotlib/_mathtext.py
Outdated
Comment on lines
2496
to
2502
| 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] |
Member
There was a problem hiding this comment.
Optional alternative spelling. Choose whichever you like.
Suggested change
| 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])] |
oscargus
approved these changes
Nov 2, 2025
Contributor
Author
|
Testing for the space is already provided by mathtext1_dejavusans_06 (after the |
timhoffm
approved these changes
Nov 2, 2025
Member
timhoffm
left a comment
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.
Contributor
Author
|
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.
Member
|
There shouldn't be any reason why it can't go on the |
QuLogic
force-pushed
the
opspace
branch
2 times, most recently
from
905fd51 to
17428e3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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