Skip to content

refactor ftface_props example - #3630

Merged
mdboom merged 3 commits into
matplotlib:masterfrom
twmr:ftprop
Oct 20, 2014
Merged

mdboom merged 3 commits into
matplotlib:masterfrom
twmr:ftprop

Conversation

@twmr

@twmr twmr commented Oct 10, 2014

Copy link
Copy Markdown
Contributor

BTW this fixes many pep8 errors in this file

@tacaswell

Copy link
Copy Markdown
Member

Not a huge fan of this change, I think it hides the bit-shift logic under a bit too much clever.

@tacaswell tacaswell added this to the unassigned milestone Oct 10, 2014
@tacaswell tacaswell modified the milestones: v1.5.x, unassigned Oct 10, 2014
@mdboom

mdboom commented Oct 10, 2014

Copy link
Copy Markdown
Member

Haven't looked at this file in a while. Note that these constants are now defined in the ft2font extension itself, so there's no need to recreate their values here. i.e., one can just do ft2font.SCALABLE.

@twmr

twmr commented Oct 10, 2014

Copy link
Copy Markdown
Contributor Author

@mdboom just found a typo in the values of the style identifiers in ft2font.cpp

What do you think about my last commit. Is it still too clever?

@tacaswell

Copy link
Copy Markdown
Member

@Thisch This needs a re-base

@mdboom Can you comment on the ft2font typo?

twmr added 3 commits October 19, 2014 11:27
Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
Signed-off-by: Thomas Hisch <t.hisch@gmail.com>
@twmr

twmr commented Oct 19, 2014

Copy link
Copy Markdown
Contributor Author

rebased

@mdboom

mdboom commented Oct 20, 2014

Copy link
Copy Markdown
Member

Thanks. The typo fixed here is legit. Thanks for catching.

I think the latest version of the example file here is much better, and not "too clever". 👍 from me.

mdboom added a commit that referenced this pull request Oct 20, 2014
refactor ftface_props example
@mdboom
mdboom merged commit 0a96efb into matplotlib:master Oct 20, 2014
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.

4 participants