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
henrybeUM wants to merge 8 commits into from
Closed

Fix spacing after mathtext operators with sub/superscripts #22839

Changes from 1 commit
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
Next Next commit
Initial Investigation into the issue and a potential fix
  • Loading branch information
@henrybeUM
henrybeUM committed Apr 13, 2022
commit c54535469b793bed8ff836a6e0c62456a9efc065
34 changes: 30 additions & 4 deletions lib/matplotlib/_mathtext.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,8 @@ def main(self, s, loc, toks):
return [Hlist(toks)]

def math_string(self, s, loc, toks):
#import pdb
#pdb.set_trace()
return self._math_expression.parseString(toks[0][1:-1])

def math(self, s, loc, toks):
Expand Down Expand Up @@ -2104,7 +2106,7 @@ def unknown_symbol(self, s, loc, toks):
r'overleftarrow': r'\leftarrow',
r'mathring': r'\circ',
}

subScriptFlag=False
_wide_accents = set(r"widehat widetilde widebar".split())

# make a lambda and call it to get the namespace right
Expand Down Expand Up @@ -2159,10 +2161,17 @@ 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 |= {'^', '_'}
print("don't stop me now")
#breakpoint()
if (next_char not in delimiters and

@anntzer anntzer Apr 16, 2022

Copy link
Copy Markdown
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.

@henrybeUM henrybeUM Apr 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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.

@henrybeUM henrybeUM Apr 21, 2022

Copy link
Copy Markdown
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?

@anntzer anntzer Apr 21, 2022

Copy link
Copy Markdown
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.

@henrybeUM henrybeUM Apr 22, 2022

Copy link
Copy Markdown
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'\,'])]
print("insertSpace")

if(next_char in {'^', '_'}):
self.subScriptFlag=True

self.pop_state()
return Hlist(hlist_list)

Expand Down Expand Up @@ -2355,7 +2364,7 @@ def subsuper(self, s, loc, toks):
constants.delta_integral) * lc_height
else:
subkern = 0

#breakpoint()
if super is None:
# node757
x = Hlist([Kern(subkern), sub])
Expand Down Expand Up @@ -2394,8 +2403,25 @@ def subsuper(self, s, loc, toks):

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

#by this point in the code, the exponent has been mounted to the nucleus
# before you return Hlist, you need to add a space to hlist to add spacing
# this will work better than the previous solution, but suffers from the
#same issue, where $sin^{23}$ will technically have a space after this
#the ideal solution would involve setting a flag for it, but
# would require knowledge of what exponent level we are actually on
#maybe, just maybe, we could set some kerning flag in Hlist
#, but honestly, I don't think its too big of a deal
# of course, we will still need to add spaces after non exponentiated operators
#@HENRY TODO
#another issue: sin^{sqrt^sin}3 will have extra space compared to
# sin^{sqrt} 3 becuase the exponent's exponent will have extra spacing
result = [nucleus, x]
if(self.subScriptFlag):
result += [self._make_space(self._space_widths[r'\,'])]
self.subScriptFlag=False
print("hopethis worked")
result = Hlist(result)
#breakpoint()
return [result]

def _genfrac(self, ldelim, rdelim, rule, style, num, den):
Expand Down