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

Switch to requiring Pillow rather than having our own png wrapper? #15165

Closed
anntzer opened this issue Aug 31, 2019 · 6 comments · Fixed by #15193
Closed

Switch to requiring Pillow rather than having our own png wrapper? #15165

anntzer opened this issue Aug 31, 2019 · 6 comments · Fixed by #15193
Milestone

Comments

@anntzer
Copy link
Contributor

anntzer commented Aug 31, 2019

It looks like Pillow's png handling (which is hand-written https://github.com/python-pillow/Pillow/blob/master/src/PIL/PngImagePlugin.py, not based on libpng) is faster than matplotlib's, both for writing and for reading:

from time import perf_counter
import matplotlib as mpl; mpl.use("agg")
from matplotlib import pyplot as plt
import numpy as np
from PIL import Image

fig, ax = plt.subplots()
fig.canvas.draw()
t = np.asarray(fig.canvas.renderer._renderer)
for writer in ["mpl", "pil"]:
    print(f"write with {writer}")
    start = perf_counter()
    for _ in range(100):
        mpl.image.imsave("/tmp/test.png", t,
                         pil_kwargs={"mpl": None, "pil": {}}[writer])
    print("write", perf_counter() - start)
    start = perf_counter()
    for _ in range(100):
        mpl.image.imread("/tmp/test.png")
    print("mpl read", perf_counter() - start)
    start = perf_counter()
    for _ in range(100):
        with Image.open("/tmp/test.png") as image:
            mpl.image.pil_to_array(image)
    print("pil read", perf_counter() - start)

yields, for me:

write with mpl
write 1.5242545299988706
mpl read 0.9935761439992348
pil read 0.46841289500298444
write with pil
write 1.400021340996318
mpl read 0.9913013100012904
pil read 0.4612834110012045

#15160 reminded me that it's not always trivial to get C-level dependencies set up before building (though that specific issue is about freetype). We already vendor agg and qhull, and have an option to download freetype (via local_freetype/MPLLOCALFREETYPE), so the sole remaining dependency that always needs to be provided by the builder is libpng.

If we just depended (as an install_requires) on Pillow for png handling instead, we would be able to drop that dependency (together with a bunch of C code). Pillow has wheels for all three major OSes, supports PyPy, is actively developed, and has been around for a very long time.

Thoughts on making the switch?

@jklymak
Copy link
Member

jklymak commented Aug 31, 2019

The idea seems reasonable. Is the performance at all dependent on the complexity of the plots, or the size of the pngs?

@anntzer
Copy link
Contributor Author

anntzer commented Sep 1, 2019

I don't have the numbers yet, but can set up more benchmarking.

@tacaswell tacaswell added this to the v3.3.0 milestone Sep 2, 2019
@tacaswell
Copy link
Member

👍 if this is going to make both the build process simpler and get us a factor of 2 in writing.

@anntzer
Copy link
Contributor Author

anntzer commented Sep 2, 2019

in reading, not in writing.

@tacaswell
Copy link
Member

oh, derp, I apparently can not read, but even a small speed up, in conjunction with the easier install is still a win.

@anntzer
Copy link
Contributor Author

anntzer commented Sep 4, 2019

Right now this is stuck on a few upstream issues (python-pillow/Pillow#3041, imageio/imageio#475); may end up needing to vendor some code from imageio with some more fixes to fix that.

On the other hand the (unpublished) draft patch I have does indeed make building on Windows much easier.

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 a pull request may close this issue.

3 participants