Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Fix spacing in r"$\max f$".#30715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
timhoffm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM, still some tests are failing.
lib/matplotlib/_mathtext.py Outdated
| result=Hlist([ | ||
| vlt, | ||
| *([self._make_space(self._space_widths[r'\,'])] | ||
| ifself._needs_space_after_subsuperelse []), | ||
| ]) | ||
| self._needs_space_after_subsuper=False | ||
| return [result] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Optional alternative spelling. Choose whichever you like.
| result=Hlist([ | |
| vlt, | |
| *([self._make_space(self._space_widths[r'\,'])] | |
| ifself._needs_space_after_subsuperelse []), | |
| ]) | |
| self._needs_space_after_subsuper=False | |
| return [result] | |
| spacing_nodes= ( | |
| [self._make_space(self._space_widths[r'\,'])] | |
| ifself._needs_space_after_subsuperelse [] | |
| ) | |
| self._needs_space_after_subsuper=False | |
| return [Hlist([vlt,*spacing_nodes])] |
oscargus left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Possibly update *_29 to include a text after? f(x)? To establish that there should be a space etc.
anntzer commentedNov 2, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Testing for the space is already provided by mathtext1_dejavusans_06 (after the |
timhoffm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'll leave this open, so that you can decide yourself on which branch this should go.
anntzer commentedNov 3, 2025
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 bemissing in expressions like `r"$\max f$"`. The difference arose becauseof the slightly different handling of subscripts and superscriptsafter the `\sin` and `\max` operators: `\sin^n` puts the superscript asa normal exponent, but `\max_x` puts the subscript centered below theoperator name ("overunder symbol). The previous code for inserting thethin space did not handle the "overunder" case; fix that. The newbehavior is tested by the change in test_operator_space, as well as bymathtext1_dejavusans_06.The change in mathtext_foo_29 arises because the extra thin space nowinserted after `\limsup` slightly shifts the centering of the wholestring. Ideally that thin space should be suppressed if there's notoken after the operator, but that's not something currently implementedeither 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 justmakes 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_nucleusvariable.
QuLogic commentedNov 6, 2025
There shouldn't be any reason why it can't go on the |
905fd51 to17428e3Comparee952f23 intomatplotlib:text-overhaulUh oh!
There was an error while loading.Please reload this page.
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