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

Labels do not becomes color anymore in figure options panel for qt toolb... #4304

Merged
merged 1 commit into from Apr 3, 2015

Conversation

Acanthostega
Copy link
Contributor

Filtering by labels is sufficient, not clean, but working.

Are you interested in making a rigid but clean widget to avoid such problems in the future ? If yes, I can probably do it in two or three weeks, when I will have sufficient time.

@tacaswell tacaswell added this to the next point release milestone Apr 1, 2015
@@ -61,6 +61,8 @@

import datetime

BLACKLIST = ["Title", "Label"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make these all lower case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also use a set, trivial performance gain...

@Acanthostega
Copy link
Contributor Author

Ok, added the set improvement. Better if the blacklist increases one day ! But the label.lower() seems to cost a lot too. Maybe check against the "brute" label is better ?

@tacaswell
Copy link
Member

The performance optimizations on the set was a bit of bike shedding which you indulged me in ;)

In most cases, it is simpler to normalize strings before checking them for being in the set rather than including all permutations of capitalization you want to support.

tacaswell added a commit that referenced this pull request Apr 3, 2015
BUG : never treat title or label values as colors in qt_editor

fixes #4303
@tacaswell tacaswell merged commit 94e6944 into matplotlib:master Apr 3, 2015
@tacaswell
Copy link
Member

@Acanthostega Thank you!

I think this is your first mpl contribution, congratulation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants