Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: re-order symbol and acent in mathtext #4588

Merged
merged 2 commits into from Jul 31, 2015

Conversation

tacaswell
Copy link
Member

Try to find symbols before finding accents

Closes #4462

attn @mdboom What can of worms have I opened here?

@mdboom
Copy link
Member

mdboom commented Jul 6, 2015

I suspect this change is fine as long as there are no symbols that are strict prefixes of accents. (Don't know if that's the case). Maybe add some tests? (Clearly it doesn't break existing ones, which is a good thing.)

@tacaswell tacaswell modified the milestone: next point release Jul 17, 2015
@tacaswell
Copy link
Member Author

@mdboom rebased + tests added

@tacaswell
Copy link
Member Author

Ping me if I should merge these tests into one, the idea was to split them up to make bisection easier

@tacaswell
Copy link
Member Author

ping @mdboom

mdboom added a commit that referenced this pull request Jul 31, 2015
FIX: re-order symbol and acent in mathtext
@mdboom mdboom merged commit bf6291b into matplotlib:master Jul 31, 2015
@tacaswell tacaswell deleted the fix_math_order branch July 31, 2015 22:09
@zblz
Copy link
Member

zblz commented Aug 7, 2015

@tacaswell: In my system this switch messes up the position of the accents. It might be system dependent, but I think my system is common enough that this shouldn't happen (everything from Debian Testing repos except matplotlib master, Python 3.4). Before the fix, accents rendered fine, symbols didn't:
accent_test_mpl_cm_master

After it, accents are shifted to the left:
accent_test_mpl_cm_old

I don't know much about parsers, so I don't really understand how this can happen from a cursory glance at mathtext.py. What other info do you need?

@tacaswell
Copy link
Member Author

Well, that is exciting.

On Fri, Aug 7, 2015, 12:19 PM Victor Zabalza notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell: In my system this switch
messes up the position of the accents. It might be system dependent, but I
think my system is common enough that this shouldn't happen (everything
from Debian Testing repos except matplotlib master, Python 3.4). Before the
fix, accents rendered fine, symbols didn't:
[image: accent_test_mpl_cm_old]
https://cloud.githubusercontent.com/assets/576258/9140290/e531d806-3d27-11e5-8022-8d2222c9d6d0.png

After it, accents are shifted to the left:
[image: accent_test_mpl_cm_master]
https://cloud.githubusercontent.com/assets/576258/9140292/ee9dbb3a-3d27-11e5-8fa9-12e87b192b57.png

I don't know much about parsers, so I don't really understand how this can
happen from a cursory glance at mathtext.py. What other info do you need?


Reply to this email directly or view it on GitHub
#4588 (comment)
.

@WeatherGod
Copy link
Member

What is the mathtex string you are using?

On Fri, Aug 7, 2015 at 12:24 PM, Thomas A Caswell notifications@github.com
wrote:

Well, that is exciting.

On Fri, Aug 7, 2015, 12:19 PM Victor Zabalza notifications@github.com
wrote:

@tacaswell https://github.com/tacaswell: In my system this switch
messes up the position of the accents. It might be system dependent, but
I
think my system is common enough that this shouldn't happen (everything
from Debian Testing repos except matplotlib master, Python 3.4). Before
the
fix, accents rendered fine, symbols didn't:
[image: accent_test_mpl_cm_old]
<
https://cloud.githubusercontent.com/assets/576258/9140290/e531d806-3d27-11e5-8022-8d2222c9d6d0.png

After it, accents are shifted to the left:
[image: accent_test_mpl_cm_master]
<
https://cloud.githubusercontent.com/assets/576258/9140292/ee9dbb3a-3d27-11e5-8fa9-12e87b192b57.png

I don't know much about parsers, so I don't really understand how this
can
happen from a cursory glance at mathtext.py. What other info do you need?


Reply to this email directly or view it on GitHub
<
https://github.com/matplotlib/matplotlib/pull/4588#issuecomment-128752230>
.


Reply to this email directly or view it on GitHub
#4588 (comment)
.

@zblz
Copy link
Member

zblz commented Aug 7, 2015

The top one is from a test, the bottom is the one from issue #4462, and which is in tests as well since this PR:

r'$\ddot{o}\acute{e}\grave{e}\hat{O}\breve{\imath}\tilde{n}\vec{q}$'
r'$\Cup \Cap \dotplus \barwedge \doteq \doteqdot \rightharpoonup \ddots \leftharpoonup$'

Note as well that the mathtext test for the first one doesn't fail for me: maybe the accents don't have enough area to meet the threshold?

@WeatherGod
Copy link
Member

re: accents, I bet you are right about that. We probably only test them on
normal size text, and so they aren't big enough to fail.

As for the parsing, what I think is really strange is that the \hat for the
'O' is at the midlevel, rather than raised, and the \breve is at the
midlevel as well. Makes me wonder if they are getting rendered completely
independently of the text they are operating upon?

On Fri, Aug 7, 2015 at 2:15 PM, Victor Zabalza notifications@github.com
wrote:

The top one is from a test, the bottom is the one from issue #4462
#4462, and which is in
tests as well since this PR:

r'$\ddot{o}\acute{e}\grave{e}\hat{O}\breve{\imath}\tilde{n}\vec{q}$'
r'$\Cup \Cap \dotplus \barwedge \doteq \doteqdot \rightharpoonup \ddots \leftharpoonup$'

Note as well that the mathtext test for the first one doesn't fail for me:
maybe the accents don't have enough area to meet the threshold?


Reply to this email directly or view it on GitHub
#4588 (comment)
.

@zblz
Copy link
Member

zblz commented Aug 7, 2015

It seems so: instead of being rendered in place and then shifted, it looks
as if the parser parses them independently of their associated character
and renders them before (assuming x-height for the base character?).
El dia 07/08/2015 19:35, "Benjamin Root" notifications@github.com va
escriure:

re: accents, I bet you are right about that. We probably only test them on
normal size text, and so they aren't big enough to fail.

As for the parsing, what I think is really strange is that the \hat for the
'O' is at the midlevel, rather than raised, and the \breve is at the
midlevel as well. Makes me wonder if they are getting rendered completely
independently of the text they are operating upon?

On Fri, Aug 7, 2015 at 2:15 PM, Victor Zabalza notifications@github.com
wrote:

The top one is from a test, the bottom is the one from issue #4462
#4462, and which is in
tests as well since this PR:

r'$\ddot{o}\acute{e}\grave{e}\hat{O}\breve{\imath}\tilde{n}\vec{q}$'
r'$\Cup \Cap \dotplus \barwedge \doteq \doteqdot \rightharpoonup \ddots
\leftharpoonup$'

Note as well that the mathtext test for the first one doesn't fail for
me:
maybe the accents don't have enough area to meet the threshold?


Reply to this email directly or view it on GitHub
<
https://github.com/matplotlib/matplotlib/pull/4588#issuecomment-128785905>
.


Reply to this email directly or view it on GitHub
#4588 (comment)
.

@tacaswell
Copy link
Member Author

Turns out this PR breaks the accent parsing, as in it never happens (and the comment in the code was correct). I am working on fixing this.

@zblz Thanks for catching this before we released!

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Aug 8, 2015
This is a follow up to matplotlib#4588

The change in ae91e9f fixed
the symbols that started with accent names, but broke all of
the other accents.

This fixes both by special casing the 8 named symbols that start with
an accent as a prefix.

ex  '\doteq' should be parsed as a single symbol, not as as two symbols
(e, q) with a dot over the e ('\dot{e}q')
@tacaswell tacaswell mentioned this pull request Aug 8, 2015
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Aug 29, 2015
This is a follow up to matplotlib#4588

The change in ae91e9f fixed
the symbols that started with accent names, but broke all of
the other accents.

This fixes both by special casing the 8 named symbols that start with
an accent as a prefix.

ex  '\doteq' should be parsed as a single symbol, not as as two symbols
(e, q) with a dot over the e ('\dot{e}q')
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants