FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix spacing after mathtext operators with sub/superscripts by henrybeUM · Pull Request #22839 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

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
Conversation 19 Commits 8 Checks 0 Files changed
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 .pdf  (5) .png  (5) .py  (2) .svg  (5) All 4 file types selected
Deleted files Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
22 changes: 21 additions & 1 deletion lib/matplotlib/_mathtext.py
Show comments
View file
Open in desktop
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
Show hidden characters
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

timhoffm Apr 25, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 |= {'^', '_'}

anntzer Apr 23, 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

undo this blank line?
if (next_char not in delimiters and

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@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
Show comments
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_cm_30.png
Show comments
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
Show comments
View file
Open in desktop

This file was deleted.

Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavusans_30.pdf
Show comments
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavusans_30.png
Show comments
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
Show comments
View file
Open in desktop

This file was deleted.

Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavuserif_30.pdf
Show comments
View file
Open in desktop
Binary file not shown.
Binary file removed lib/matplotlib/tests/baseline_images/test_mathtext/mathtext_dejavuserif_30.png
Show comments
View file
Open in desktop
Binary file not shown.
Loading
Back | FazBrowse Home | New Git URL