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

use _fast_from_codes_and_verts in transform code #3681

Merged
merged 1 commit into from
Oct 20, 2014

Conversation

jbmohler
Copy link
Contributor

This commit uses the internal constructor for Path objects. This results in a 4% speedup during plot drawing.

@tacaswell
Copy link
Member

Do you have any idea what the test coverage for this change is like?

Sorry, my new hobby horse is test coverage after the boxplot refactor introduces ~5 major regressions.

@jenshnielsen
Copy link
Member

@tacaswell perhaps we should consider setting up something like https://coveralls.io/ with the matplotlib travis to get a feel for the coverage?

@tacaswell
Copy link
Member

I have looked at the coverage briefly and it is a depressing situation (like 60% depressing). Setting up coveralls is a fine idea (so long as we keep it from posting on every PR thread on every commit).

@jbmohler
Copy link
Contributor Author

@tacaswell I fully support the coverage hobby horse in general ... the caveat being that I'm already busy beating coverage into submission on my main project :). Coverage monitoring from travis would be really sweet -- I would expect that part of travis' output should be if coverage decreases on a particular PR. I find that coverage's html output is plenty good enough for interactive inspection of it.

Speaking of this PR in particular: Both code hunks are covered in the test run on my linux machine. I couldn't speak intelligently as to whether there are multiple diverse states in which coverage is necessary here.

@pelson
Copy link
Member

pelson commented Oct 20, 2014

I just wanted to add that I've tested this against cartopy (which is a big user of the affine/non-affine separation) and there seems to be nothing out of the ordinary (cartopy tests don't actually pass with >=v1.4.0) therefore I'm 👍 on this.

Nice little optimisation!

@tacaswell
Copy link
Member

@pelson What did we break that breaks the cartopy tests?

tacaswell added a commit that referenced this pull request Oct 20, 2014
PRF : use _fast_from_codes_and_verts in transform code
@tacaswell tacaswell merged commit c299457 into matplotlib:master Oct 20, 2014
@jbmohler jbmohler deleted the use_fast_from_codes_and_verts branch October 20, 2014 19:24
@pelson
Copy link
Member

pelson commented Oct 21, 2014

What did we break that breaks the cartopy tests?

Ah, we just moved a few clip areas - nothing major (and that wasn't picked up in mpl's tests) just not pixel-pixel perfect between v1.3 and v1.4.

tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Jan 17, 2015
…m_codes_and_verts"

This reverts commit c299457, reversing
changes made to 38d69b0.
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.

5 participants