Simplify _png extension by handling file open/close in Python. #15095
Conversation
Make functions in the (private) _png extension only take file-like objects as arguments, rather than "filename or file-like". Filenames were anyways handled by calling the python C-API to open/close the files, so we may as well just do this from Python where a simple `with open(...):` does the job. The public API is not changed.
| @@ -639,7 +639,8 @@ def draw_image(self, gc, x, y, im, transform=None): | |||
| fname = os.path.splitext(os.path.basename(self.fh.name))[0] | |||
| fname_img = "%s-img%d.png" % (fname, self.image_counter) | |||
| self.image_counter += 1 | |||
| _png.write_png(im[::-1], os.path.join(path, fname_img)) | |||
| with pathlib.Path(path, fname_img).open("wb") as file: | |||
There was a problem hiding this comment.
I don't quite like assigning to builtin names, but acknowledge that there's already precedence for this in the library.
There was a problem hiding this comment.
It's not a builtin name anymore...
Even though the public API hasn't changed, i think this does seem to change the error types raised and has broken downstream code in SymPy. See this issue: sympy/sympy#18815 The offending changed lines are: https://github.com/matplotlib/matplotlib/blame/v3.2.0/lib/matplotlib/mathtext.py#L3436 I haven't determined yet if it is ultimately SymPy's bad design for relying on a specific error but this PR might cause all Jupyter notebook printing for SymPy to break once people install matplotlib 3.2.0. |
|
Can you try with the 3.2.x branch? We just merged #16709 which I think is related. |
|
Yes, this is the same bug; see backtrace on linked issue. |
Make functions in the (private) _png extension only take file-like
objects as arguments, rather than "filename or file-like". Filenames
were anyways handled by calling the python C-API to open/close the
files, so we may as well just do this from Python where a simple
with open(...):does the job. The public API is not changed.PR Summary
PR Checklist