Skip to content

Fix spacing after mathtext operators with sub/superscripts #22839

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

Fix spacing after mathtext operators with sub/superscripts #22839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c545354
Initial Investigation into the issue and a potential fix
henrybeUM Apr 13, 2022
99d0801
Merge branch 'matplotlib:main' into bugfix-for-issue-17852
henrybeUM Apr 13, 2022
a96e530
Cleaned up _mathtext.py comments, added relavant testing
henrybeUM Apr 13, 2022
c9c5e62
Merge branch 'bugfix-for-issue-17852' of https://github.com/henrybeUM…
henrybeUM Apr 13, 2022
c99596e
updated test images for mathtext_rendering, modified comments and fla…
henrybeUM Apr 15, 2022
cc3e7aa
Updated style of mathtext contributions
henrybeUM Apr 15, 2022
9b36a78
Deleted test 30, also fixed a bug where subsuper state could leak int…
henrybeUM Apr 22, 2022
6b39a68
Removed two empty lines to pass linting
henrybeUM Apr 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion lib/matplotlib/_mathtext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1947,6 +1947,9 @@ def __init__(self):
self._expression = p.main
self._math_expression = p.math

# To add space to nucleus operators after sub/superscripts
self._subsuper_flag = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly clearer naming:

Suggested change
self._subsuper_flag = False
self._in_subscript_or_superscript = False


def parse(self, s, fonts_object, fontsize, dpi):
"""
Parse expression *s* using the given *fonts_object* for
Expand All @@ -1965,6 +1968,8 @@ def parse(self, s, fonts_object, fontsize, dpi):
" " * (err.column - 1) + "^",
str(err)])) from err
self._state_stack = None
self._subsuper_flag = False
# prevent operator spacing from leaking into a new expression
self._em_width_cache = {}
self._expression.resetCache()
return result[0]
Expand Down Expand Up @@ -2159,11 +2164,19 @@ def operatorname(self, s, loc, toks):
next_char = next((c for c in s[next_char_loc:] if c != ' '), '')
delimiters = self._left_delim | self._ambi_delim | self._right_delim
delimiters |= {'^', '_'}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undo this blank line?

if (next_char not in delimiters and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it looks like avoiding insertion of the spurious space is just adding a check that if (next_char != '' and ...) (i.e. we are not at the end of the math string). Perhaps you can make that change as part of this PR too? This way we would avoid changing the baseline images.

Copy link
Contributor Author

@henrybeUM henrybeUM Apr 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer, I don't think this actually fixes the issue. I made the change, reverted to the original images locally, and the same failures occur. I believe changing this statement could alter spacing after non sub/super scripted operators. However, not for sub/superscripted ones, since they will be followed by a '_' or '^' by definition.

In essence, we would need to check the next char after the sub/superscripted content, but it's very hard to tell where the sub/super scripted content stops.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer any thoughts on how you want to move forward?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, indeed the above solution is not enough. Perhaps we can just drop that test for now (replace the entry by None, and delete the baseline images): indeed, there's already tests 37, 56, and 75 which test placement of integrals limits, which seems enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anntzer I dropped the test and made a correction to the initial code to reset the flag between queries, It should be good to go. :)

toks[0] not in self._overunder_functions):
# Add thin space except when followed by parenthesis, bracket, etc.
hlist_list += [self._make_space(self._space_widths[r'\,'])]
self.pop_state()
# if followed by a super/subscript, set flag to true
# This flag tells subsuper to add space after this operator
if next_char in {'^', '_'}:
self._subsuper_flag = True
else:
self._subsuper_flag = False

return Hlist(hlist_list)

def start_group(self, s, loc, toks):
Expand Down Expand Up @@ -2394,8 +2407,15 @@ def subsuper(self, s, loc, toks):

if not self.is_dropsub(last_char):
x.width += constants.script_space * xHeight
result = Hlist([nucleus, x])

# Do we need to add a space after the nucleus?
# To find out, check the flag set by operatorname
spaced_nucleus = [nucleus, x]
if self._subsuper_flag:
spaced_nucleus += [self._make_space(self._space_widths[r'\,'])]
self._subsuper_flag = False

result = Hlist(spaced_nucleus)
return [result]

def _genfrac(self, ldelim, rdelim, rule, style, num, den):
Expand Down
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_cm_30.pdf
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_cm_30.png
View file
Open in desktop
Binary file not shown.
188 changes: 0 additions & 188 deletions lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_cm_30.svg
View file
Open in desktop

This file was deleted.

Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavusans_30.pdf
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavusans_30.png
View file
Open in desktop
Binary file not shown.
124 changes: 0 additions & 124 deletions lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavusans_30.svg
View file
Open in desktop

This file was deleted.

Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavuserif_30.pdf
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavuserif_30.png
View file
Open in desktop
Binary file not shown.
Loading