Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
boldsymbol support for mathtext#25661
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
Uh oh!
There was an error while loading.Please reload this page.
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.
| To change font to bold and italic enclose the text in a font command as | ||
| shown: | ||
| .. code-block:: |
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 do not fully get this warning, but I guess it must be fixed one way or the other. Maybe better to add a figure in the other PR, likeplt.text(...) and then add the alternative version in that figure here?
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.
Currently, I am getting these results from the current implementation. Should I add themathbfit to the other PR, and theboldsymbol to the current one?
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 think it is just missing a blank line.
.. code-block:: r'$\boldsymbol{a+2+\alpha}$'(note also that the closing quote is missing anyway)
| hlist.append(c) | ||
| else: | ||
| hlist.append(c) | ||
| self.pop_state() |
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.
Didn't really check, but does that work with nesting? (i.e.\boldsymbol{\mathnormal{foo}} -- the inner one should win)
I'm not actually sure what the semantics of boldsymbol are in TeX, but shouldn't it just be an alias for mathbfit? (and perhaps the test could be based on check_figures_equal too)
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.
Thecheck_figures_equal test in the PR currently gives this output:
7371e42 to1447b6dCompareThe PR linked above has been merged, is this still waiting on anything? |
Hi@ksunden, This PR is not waiting, I opened it for review. Thanks! |
Uh oh!
There was an error while loading.Please reload this page.
| fig_test.text(0.1, 0.1, r"$\boldsymbol{abc0123\alpha}$") | ||
| fig_test.text(0.1, 0.2, r"$\boldsymbol{\mathrm{abc0123\alpha}}$") | ||
| fig_ref.text(0.1, 0.1, r"$\boldsymbol{abc0123\alpha}$") |
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 think this should use \mathbfit, no?
anntzer commentedMay 2, 2023 • 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.
I'm sorry for doing this review in a piecemeal fashion, but I think that we still don't know exactly what are the semantics of boldsymbol in tex and should thus figure this out exactly first. |
After exploring a bit on I generated this image for (some symbols weren't rendered properly, I'm guessing due to LaTeX package differences) This is the link that I used for reference.http://tug.ctan.org/tex-archive/macros/amstex/doc/amsguide.pdf |
lib/matplotlib/_mathtext.py Outdated
| .lower(), | ||
| range(ord('\N{GREEK SMALL LETTER ALPHA}'), | ||
| ord('\N{GREEK SMALL LETTER OMEGA}') + 1))) | ||
| allLatin = [*lw, *up] |
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.
This is equal tostring.ascii_letters.
lib/matplotlib/_mathtext.py Outdated
| for c in name: | ||
| if isinstance(c, Char): | ||
| c.font = "bf" | ||
| if str(c)[1] in allLatin or str(c)[2:-1] in smGreek: |
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.
str(c)[1:-1] is equal toc.c forChar, I think?
| ifstr(c)[1]inallLatinorstr(c)[2:-1]insmGreek: | |
| ifc.cinstring.ascii_lettersorc.c[1:]inlower_greek: |
lib/matplotlib/_mathtext.py Outdated
| smGreek = list(map(lambda a: unicodedata.name(chr(a)) | ||
| .split()[-1] | ||
| .lower(), | ||
| range(ord('\N{GREEK SMALL LETTER ALPHA}'), | ||
| ord('\N{GREEK SMALL LETTER OMEGA}') + 1))) |
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.
Should rename for PEP8, and maybe also prefer list comprehensions
| smGreek=list(map(lambdaa:unicodedata.name(chr(a)) | |
| .split()[-1] | |
| .lower(), | |
| range(ord('\N{GREEK SMALL LETTER ALPHA}'), | |
| ord('\N{GREEK SMALL LETTER OMEGA}')+1))) | |
| lower_greek= [ | |
| unicodedata.name(chr(a)).split()[-1].lower() | |
| forainrange(ord('\N{GREEK SMALL LETTER ALPHA}'), | |
| ord('\N{GREEK SMALL LETTER OMEGA}')+1))] |
6e7850e to3ac9e61Compare| Boldsymbol mathtext command ``\boldsymbol`` | ||
| ----------------------------------------------------- |
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.
Should match text length to underline.
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.
Trying the image test code out on normal LaTeX, it seems like the single + should not be bold?
I checked the |
OK! I guess my point really was that the + in Is it really different or is there just some pixel snapping issue in the test image that makes it look that way? |
devRD commentedJun 24, 2023 • 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.
Good catch! |
dd805ef to10a1ad7Comparelib/matplotlib/_mathtext.py Outdated
| if isinstance(c, Hlist): | ||
| k = c.children[1] | ||
| if isinstance(k, Char): | ||
| k.font = "bf" | ||
| k._update_metrics() | ||
| if isinstance(c, Char): | ||
| c.font = "bf" | ||
| if c.c in latin_alphabets or c.c[1:] in small_greek: | ||
| c.font = "bfit" | ||
| c._update_metrics() | ||
| c._update_metrics() | ||
| hlist.append(c) | ||
| else: | ||
| hlist.append(c) |
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.
| ifisinstance(c,Hlist): | |
| k=c.children[1] | |
| ifisinstance(k,Char): | |
| k.font="bf" | |
| k._update_metrics() | |
| ifisinstance(c,Char): | |
| c.font="bf" | |
| ifc.cinlatin_alphabetsorc.c[1:]insmall_greek: | |
| c.font="bfit" | |
| c._update_metrics() | |
| c._update_metrics() | |
| hlist.append(c) | |
| else: | |
| hlist.append(c) | |
| ifisinstance(c,Hlist): | |
| k=c.children[1] | |
| ifisinstance(k,Char): | |
| k.font="bf" | |
| k._update_metrics() | |
| elifisinstance(c,Char): | |
| c.font="bf" | |
| ifc.cinlatin_alphabetsorc.c[1:]insmall_greek: | |
| c.font="bfit" | |
| c._update_metrics() | |
| c._update_metrics() | |
| hlist.append(c) |
lib/matplotlib/_mathtext.py Outdated
| state = self.get_state() | ||
| small_greek = [unicodedata.name(chr(i)).split()[-1].lower() for i in | ||
| range(ord('\N{GREEK SMALL LETTER ALPHA}'), | ||
| ord('\N{GREEK SMALL LETTER OMEGA}') + 1)] |
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.
Maybe one should move this to be a member of the parser class so that it is not recreated every time?
(And considering that it is only used for testing membership, make it aset?)
Nice that you got it to work! Just some minor suggestions for clarity and performance. |
2cfd449 to05a3261Compare
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Fixes#25643
Fixes#1366
Depends on PR#25359
Add boldsymbol command
PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst