8000 fixed bug 6028 by crazyo · Pull Request #6073 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

fixed bug 6028 #6073

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Mar 6, 2016
Merged

fixed bug 6028 #6073

merged 4 commits into from
Mar 6, 2016

Conversation

crazyo
Copy link
Contributor
@crazyo crazyo commented Feb 29, 2016

The problem was that hyphen-minus was converted to minus signs without differentiating those in math expressions and those in texts. I fixed it by adding an optional parameter nonMath so that hyphens in texts won't be converted.

@crazyo crazyo mentioned this pull request Feb 29, 2016
Closed
@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Feb 29, 2016
@tacaswell
Copy link
Member

@mdboom @zblz

@mdboom
Copy link
Member
mdboom commented Feb 29, 2016

Thanks for investigating the issue.

I think actually, for the non-math context, get_unicode_index should always return ord(symbol) and do nothing else. The fact that it could fall through to return tex2uni[symbol.strip("\\")] is also a bug. So maybe add:

if non_math:
    return ord(symbol)

to the top and leave the rest of get_unicode_index as is.

Also, to be PEP8-compliant, the new keyword argument should be spelled non_math, not nonMath.

8000 @crazyo
Copy link
Contributor Author
crazyo commented Feb 29, 2016

@mdboom Hi thank you for your response. I have made changes accordingly :-)

@mdboom
Copy link
Member
mdboom commented Feb 29, 2016

👍 from me, but I'll leave this up for a little while in case others have comments.

@zblz
Copy link
Member
zblz commented Mar 1, 2016

Looks good to me, even though the code might be clearer if a math flag instead of non_math was used. Double negatives like non_math=False make understanding the code a little bit more difficult.

@crazyo
Copy link
Contributor Author
crazyo commented Mar 1, 2016

@zblz I've made the changes :-)

tacaswell added a commit that referenced this pull request Mar 6, 2016
@tacaswell
Merge pull request #6073 from crazyo/master
85089a1 
fixed #6028
@tacaswell tacaswell merged commit 85089a1 into matplotlib:master Mar 6, 2016
@tacaswell
Copy link
Member

@crazyo Can you open a new PR to add a simple test for this?

tacaswell added a commit that referenced this pull request Mar 6, 2016
@tacaswell
Merge pull request #6073 from crazyo/master
ebfbc4a 
fixed #6028
@tacaswell
Copy link
Member

backported to v2.x as ebfbc4a

@crazyo
Copy link
Contributor Author
crazyo commented Mar 7, 2016

yes, will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
topic: text
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@crazyo @tacaswell @mdboom @zblz @QuLogic
0