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
Fixes for File Saving in Webagg #3981
Conversation
@@ -197,7 +197,13 @@ def get(self, fignum, fmt): | |||
|
|||
self.set_header('Content-Type', mimetypes.get(fmt, 'binary')) | |||
|
|||
buff = io.BytesIO() | |||
# override fileno to raise AttributeError so PIL doesn't error | |||
class BytesIO(io.BytesIO): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be laundered through six? https://pythonhosted.org/six/#six.BytesIO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that might actually solve both problems...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, pgf
would still require some gymnastics (I don't think its worth it).
Tested with Python 3.4 and 2.7. |
@@ -403,7 +403,8 @@ def get_javascript(cls, stream=None): | |||
for filetype, ext in sorted(FigureCanvasWebAggCore. | |||
get_supported_filetypes_grouped(). | |||
items()): | |||
extensions.append(ext[0]) | |||
if not ext[0] == 'pgf': # pgf does not support BytesIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record @pwuertz.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point I added support for writing to targets that are matplotlib.cbook.is_writable_file_like
. Yet writing pgf
instead of pdf
or png
to a stream raises a UserWarning since there is no way of embedding rastered images within PGF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or are you saying that the backend currently doesn't work with BytesIO instances at all? If so, we should open a new issue and I will look into that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible, I think it's preferable to fix this at the source (having pgf
support BytesIO objects, if that's possible), rather than papering around it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this should be back ported
On Thu, Jan 8, 2015, 07:28 Michael Droettboom notifications@github.com
wrote:
In lib/matplotlib/backends/backend_webagg_core.py
#3981 (diff)
:@@ -403,7 +403,8 @@ def get_javascript(cls, stream=None):
for filetype, ext in sorted(FigureCanvasWebAggCore.
get_supported_filetypes_grouped().
items()):
extensions.append(ext[0])
if not ext[0] == 'pgf': # pgf does not support BytesIO
If possible, I think it's preferable to fix this at the source (having pgf
support BytesIO objects, if that's possible), rather than papering around
it here.—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3981/files#r22648899.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backporting is fine, but what I was getting at is that it's probably a bug that pgf
doesn't support BytesIO, and we should fix that rather than just not allowing it in the NbAgg
interface. Though this is fine for now, we'll probably want to undo it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That comment went someplace very surprising...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new issue #3982 to track the underlying problem.
Awesome, thanks @blink1073. |
@tacaswell - I think we should backport this. Do you agree? |
backported as 4c12d81 |
Closes #3979. Uses
six.BytesIO
for compatibility, which prevents a bug when usingPIL
(notPillow
) related toio.BytesIO
. Also, thepgf
backend relies on writing encoded text to a file, which is incompatible withBytesIO
.