Fix mathtext mismatched braces#26519
Conversation
|
We discussed this a bit on GSOC call, the error message is not the most useful, but it is actually pretty hard to get a better one (the Current feeling is that erroring is better than not, but if someone has an idea of how to get the parser to give a more informative message, I'd be open to hearing it. |
|
Actually, this diff may get us a better error message: diff --git a/lib/matplotlib/_mathtext.py b/lib/matplotlib/_mathtext.py
index 25a825b7b0..de4c25bfe5 100644
--- a/lib/matplotlib/_mathtext.py
+++ b/lib/matplotlib/_mathtext.py
@@ -1894,6 +1894,7 @@ class Parser:
p.function = csnames("name", self._function_names)
p.group = p.start_group + ZeroOrMore(p.token)("group") + p.end_group
+ p.unclosed_group = p.start_group + ZeroOrMore(p.token)("group") + StringEnd()
p.frac = cmd(r"\frac", p.required_group("num") + p.required_group("den"))
p.dfrac = cmd(r"\dfrac", p.required_group("num") + p.required_group("den"))
@@ -1943,6 +1944,7 @@ class Parser:
p.simple
| p.auto_delim
| p.unknown_symbol # Must be last
+ | p.unclosed_group # Must be last
)
p.operatorname = cmd(r"\operatorname", "{" + ZeroOrMore(p.simple)("name") + "}")
@@ -2144,6 +2146,9 @@ class Parser:
def unknown_symbol(self, s, loc, toks):
raise ParseFatalException(s, loc, f"Unknown symbol: {toks['name']}")
+ def unclosed_group(self, s, loc, toks):
+ raise ParseFatalException(s, len(s), "Expected '}'")
+
_accent_map = {
r'hat': r'\circumflexaccent',
r'breve': r'\combiningbreve',
diff --git a/lib/matplotlib/tests/test_mathtext.py b/lib/matplotlib/tests/test_mathtext.py
index d80312495d..5a9e8a8b92 100644
--- a/lib/matplotlib/tests/test_mathtext.py
+++ b/lib/matplotlib/tests/test_mathtext.py
@@ -320,6 +320,7 @@ def test_fontinfo():
(r'$a^2^2$', r'Double superscript'),
(r'$a_2_2$', r'Double subscript'),
(r'$a^2_a^2$', r'Double superscript'),
+ (r'$a = {b$', r"Expected '}'"),
],
ids=[
'hspace without value',
@@ -347,7 +348,8 @@ def test_fontinfo():
'unknown symbol',
'double superscript',
'double subscript',
- 'super on sub without braces'
+ 'super on sub without braces',
+ 'unclosed group',
]
)
def test_mathtext_exceptions(math, msg):@devRD thoughts? |
|
I think the diff makes sense, we do get a better error message. Added the changes... |
lib/matplotlib/_mathtext.py
Outdated
| )("sym").leaveWhitespace() | ||
| p.unknown_symbol = Regex(r"\\[A-Za-z]+")("name") | ||
| def unclosed_group(self, s, loc, toks): | ||
| raise ParseFatalException(s, len(s), "Expected '}'") |
There was a problem hiding this comment.
The location of this method is not that logical. I'd suggest moving it down to other group-methods. Otherwise, it looks good!
There was a problem hiding this comment.
Let me rephrase that: I think this should be removed as there is another method below (at a different level, so I wonder if this is used at all?).
There was a problem hiding this comment.
I think the method is being utilized because we get the same error message as above.
Re: there is another method below (at a different level - I might be missing something, could you point out which one?
|
Although not completely sure if this is correct, I think the same behavior is also achieved by redefining the Instead, the error message here would be |
lib/matplotlib/_mathtext.py
Outdated
| raise ParseFatalException(s, loc, f"Unknown symbol: {toks['name']}") | ||
|
|
||
| def unclosed_group(self, s, loc, toks): | ||
| raise ParseFatalException(s, len(s), "Expected '}'") |
There was a problem hiding this comment.
Here is the other, identical, method. Should only require one and I think this is the one.
There was a problem hiding this comment.
Yup, I missed that. Thanks!
Co-authored-by: Kyle Sunden <git@ksunden.space>
oscargus
left a comment
oscargus
left a comment
There was a problem hiding this comment.
Looks good! Not sure if it should go into 3.8 or 3.9 (I'd vote, always, for 3.8...), so I let one of the reviewers closer to the release process tag and merge.
…519-on-v3.8.x Backport PR #26519 on branch v3.8.x (Fix mathtext mismatched braces)
4 participants
PR summary
Fixes #26510
PR checklist