Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Remove *math* parameter of various mathtext internal APIs.#22507
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
How does this affect the possibility to support |
I think it's OK? Your implementation at#22173 should not be affected too much by this, as long as it doesn't perform the same dash-to-unicodeminus replacement as is now done by symbol(). |
oscargus commentedFeb 19, 2022 • 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.
OK! Just was a bit worried that the long dash was already in the string, so that Not sure I will finish#22173 (especially since I read that you have one), but we'll see. |
I don't have an implementation of Replacment to unicode minus occurs within |
OK! Just have some memory of reading that you had a branch hanging around (but I may be wrong and either I didn't read it or it was someone else). Ahh, I see now. Primarily read the description, which caused my doubt. |
I think I gave it a try at some point, but am not sure it's really ready either, so let's try to get yours in. |
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.
Not fully sure why the image tests are removed, but the general change seems to make sense.
The old image had a non-unicode minus (due to#21687); this patch changes the image. But we try to avoid adding new (or modified) baseline images if possible (due to repo size concerns); here the test was really to check what are the ticks so now I just do that instead. |
Not sure if all the failures are related.... |
Should be good to go now. |
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.
May self-merge after addressing the formatting suggestion.
Uh oh!
There was an error while loading.Please reload this page.
The *math* parameter is passed through many layers of the call stackbut is ultimately only used for a single purpose: deciding whether toreplace the ASCII hyphen by a (longer) unicode minus. Instead of doingthat, just do the substitution at the parsing stage. In particular,this fixes problematic unicode minus support with the "cm" fontset.This patch also reverts a significant part of52003e4, as LogFormattersno longer need to pass unicode minuses in mathtext -- everything getsconverted by mathtext. Likewise, this change also invalidates thetest_log_scales baseline image (old, buggy wrt. unicode minus); replaceit by a test that the drawn ticks are as expected (which was the intentin90c1aa3).
Uh oh!
There was an error while loading.Please reload this page.
Themath parameter is passed through many layers of the call stack
but is ultimately only used for a single purpose: deciding whether to
replace the ASCII hyphen by a (longer) unicode minus. Instead of doing
that, just do the substitution at the parsing stage. In particular,
this fixes problematic unicode minus support with the "cm" fontset.
This patch also reverts a significant part of52003e4, as LogFormatters
no longer need to pass unicode minuses in mathtext -- everything gets
converted by mathtext. Likewise, this change also invalidates the
test_log_scales baseline image (old, buggy wrt. unicode minus); replace
it by a test that the drawn ticks are as expected (which was the intent
in90c1aa3).
Closes#21687.
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).