Re: [Matplotlib-users] mpl-1.2.1: Speedup code by removing .startswith() calls and some for loops

[Benjamin R. <ben...@ou...>]

On Fri, Aug 9, 2013 at 9:04 AM, Martin Mokrejs
<mmo...@fo...>wrote:

> Hi Phil,
>
> Phil Elson wrote:
> > Hi Martin,
> >
> > Thanks for this - we are really interested in speeding up the scatter
> and barchart plotting with large data sets. In fact, we've done some work (
> https://github.com/matplotlib/matplotlib/pull/2156) recently to make the
> situation better.
> >
> > I'd really like to review these changes (against matplotlib master), and
> the best possible solution to doing this is if you were to submit a pull
> request. If the changes you have made are logically seperable, then I'd
> encourage you to make a few PRs, but otherwise, a single PR with all of
> these changes would be great.
>
> I went through the changes there and they just cope with other pieces of
> matplotlib.
> My changes are general python improvements moving away from
> str.startswith()
> and using generators instead of for loops. Just apply the patches yourself
> and see.
> ;)
>
> >
> > Would you mind turning these patches into PR(s)? (
> https://github.com/matplotlib/matplotlib/compare/)
>
> Um, I don't know what to do on that page, sorry. I don't see how to upload
> my patch file or patched file
> to be compared with master. :(
>
> >
> > Thanks!
>
> I am sorry but I just don't have time to fiddle with github. It is just
> awkward. I even failed to download
> diffs of the changes from
> https://github.com/matplotlib/matplotlib/pull/2156/commits.
>
> I rather continue studying runsnake output. ;-)
>
> Martin
>
>
A snippet from one of you patches:

         dsu = []

-        for a in self.patches:
-            dsu.append( (a.get_zorder(), a, a.draw, [renderer]))
+        [ dsu.append( (x.get_zorder(), x, x.draw, [renderer])) for x in
self.patches ]

Yes, we certainly should use list-comprehensions here, but you are using it
incorrectly.  It should be:

dsu = [(x.get_zorder(), x, x.draw, [renderer])) for x in self.patches ]

And then, further down, do the following:

dsu.extend((x.get_zorder(), x, x.draw, [renderer])) for x in self.lines)

Note the generator form of the comprehension as opposed to the list
comprehension form. List comprehensions should *always* be assigned to
something. List comprehensions should only be for replacing the for-append
idiom in python.

Thank you though for pointing out parts of the code that can benefit from
revisions. I certainly hope you can get this put together as a pull request
on github so we can work to make this patch better!

Ben Root

P.S. - I <3 runsnakerun!