Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Improve readability of _mathtext_data.stix_virtual_fonts table#25990
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
Improve readability of _mathtext_data.stix_virtual_fonts table#25990
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping@matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join uson gitter for real-time discussion.
For details on testing, writing docs, and our review process, please seethe developer guide
We strive to be a welcoming and open project. Please follow ourCode of Conduct.
Using ord function to convert the unicode to an integer
lib/matplotlib/_mathtext_data.py Outdated
| (0x03a3, 0x03a3, 'rm', 0x2140), # \Sigma | ||
| (0x03b3, 0x03b3, 'rm', 0x213d), # \gamma | ||
| (0x03c0, 0x03c0, 'rm', 0x213c), # \pi | ||
| (ord("\N{GREEK CAPITAL LETTER GAMMA}"), |
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 would also replace the codepoints above (0x0030 ="0" (no real need to write that as"\N{DIGIT ZERO}")), as well as the last items of the tuplewhen applicable (0x213e ="\N{DOUBLE-STRUCK CAPITAL GAMMA}" but 0x1d7d8 is just an internal codepoint for the font with no associated name).
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.
Hi, thanks for the feedback! I included replacements for these codepoints and for the last items that i could find as well in my last commit
replaced digits and letters
As noted in the issue, I don't think you want to write |
Thanks for the feedback@QuLogic! I've updated the code to include the list and dict comprehension now. For the unicodes list i had to include "\N{}" in the strings so it wouldn't throw an error when it loops because of "0", "9" and the letters strings. Btw, i really couldn't identify why there is a linting problem |
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 don't think there's a need forlist_uni oruni_map; that's basically a longer way of writingord. Rather, just write out the text in each of the tables, and do one dict/list comprehension at the end over everything instix_virtual_fonts. If you need to mix characters and ordinals, then perhaps only do that on the lists that need it directly.
PR summary
Hi! I made some changes on mathtext_data file to improve readability (using Unicode character name in order to make to code clearer) as asked (issue 25738).
So, i changed the table from like this:
To this:
PR checklist