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

Fix svg writer for StringIO objects #1380

Merged
merged 1 commit into from Oct 15, 2012
Merged

Conversation

dmcdougall
Copy link
Member

Fix #1373.

This seems like a really simple fix. I'm a little concerned that it may now be possible to pass something to print_svg that doesn't make sense. Hopefully someone will correct me if this is the case.

Also, I kind of feel like the write should be in a try/finally block, like print_png in the agg backend.

@dmcdougall
Copy link
Member Author

Also, ByteIO and StringIO both have close methods that aren't being called after the figure has been written. Perhaps I should fix this too...

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

This seems to have created a new Travis failure about seeking on a closed file. Needs further investigation.

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

Regardless of the closing, I can confirm that the initial fix does the trick. Thanks.

@dmcdougall
Copy link
Member Author

I assumed one would not seek the file once the figure has been written. Maybe I shouldn't assume this. Investigating...

@dmcdougall
Copy link
Member Author

I see the problem. The test re-reads the saved figure to test that it's a valid svg file. There are two options:

  1. Don't close the user's file descriptor. To implement this I will undo the change that closes the file descriptor and the test will pass.

  2. Assume the user wants the file descriptor closed after writing. Possibly not the correct assumption if given a specially constructed BytesIO or StringIO object. To fix this, the test will need changing to re-open the file and check its contents. The test will pass.

I think I should probably undo the close change and assume the user doesn't want specially constructed file descriptors closed. Thoughts?

@dmcdougall
Copy link
Member Author

Ok, I'm making a decision. This PR should fix the original problem and not try to fiddle with other things. I will undo the close change and keep the original solution.

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

I think that's right, having just looked at this again. I think the intent is that only things that are created within that function should be closed.

@dmcdougall
Copy link
Member Author

@mdboom Yeah. I think I'd be annoyed if functions started freeing memory when I didn't expect it. The tests pass now, too.

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

Great! Merging.

mdboom added a commit that referenced this pull request Oct 15, 2012
Fix svg writer for StringIO objects
@mdboom mdboom merged commit 940c2b9 into matplotlib:v1.2.x Oct 15, 2012
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.

None yet

2 participants