backend_ps: Do not write to a temporary file unless using an external distiller #1826

Merged
merged 1 commit into from Mar 19, 2013

Conversation

Projects
None yet
3 participants
@mgiuca-google
Contributor

mgiuca-google commented Mar 15, 2013

If ps.usedistiller is omitted, writes directly to the output file, instead of creating a temporary file and subsequently copying it to the output.

This is more efficient, and also works on restricted platforms such as Google App Engine, which are unable to provide a writable temporary file system.

Partial fix for Issue #1823.

@pelson

This comment has been minimized.

Show comment Hide comment
@pelson

pelson Mar 15, 2013

Member

I'm not sure who our resident PS expert is. @jkseppan? @mdboom - any ideas?

Member

pelson commented Mar 15, 2013

I'm not sure who our resident PS expert is. @jkseppan? @mdboom - any ideas?

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Mar 15, 2013

Owner

This looks fine to me.

Owner

mdboom commented Mar 15, 2013

This looks fine to me.

@pelson

This comment has been minimized.

Show comment Hide comment
@pelson

pelson Mar 18, 2013

Member

Is there any documentation needed for this change? Is the ps.usedistiller == False new functionality?

Member

pelson commented Mar 18, 2013

Is there any documentation needed for this change? Is the ps.usedistiller == False new functionality?

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Mar 18, 2013

Owner

I don't think it's new functionality, only that it will now write directly to a file, rather than to a temporary and then moved. I suppose if someone was relying on the old behavior that may be a problem -- but this change I think ultimately makes things less surprising.

Owner

mdboom commented Mar 18, 2013

I don't think it's new functionality, only that it will now write directly to a file, rather than to a temporary and then moved. I suppose if someone was relying on the old behavior that may be a problem -- but this change I think ultimately makes things less surprising.

backend_ps: Do not write to a temporary file unless using an external…
… distiller.

If ps.usedistiller is omitted, writes directly to the output file, instead of
creating a temporary file and subsequently copying it to the output.

This is more efficient, and also works on restricted platforms such as Google
App Engine, which are unable to provide a writable temporary file system.
@mgiuca-google

This comment has been minimized.

Show comment Hide comment
@mgiuca-google

mgiuca-google Mar 18, 2013

Contributor

To clarify, you cannot write "ps.usedistiller = False" in the matplotlibrc. When I said "is False", I meant if the Python variable is False, which it is if ps.usedistiller is omitted from the matplotlibrc. I have changed the Git commit message to clarify.

I don't see how someone could be broken by this change. There is no noticeable difference between the old and new behaviour unless you are watching the temporary directory for files being written and quickly deleted. As far as I can tell, the only difference is that it works even if there is no writable temporary directory.

Contributor

mgiuca-google commented Mar 18, 2013

To clarify, you cannot write "ps.usedistiller = False" in the matplotlibrc. When I said "is False", I meant if the Python variable is False, which it is if ps.usedistiller is omitted from the matplotlibrc. I have changed the Git commit message to clarify.

I don't see how someone could be broken by this change. There is no noticeable difference between the old and new behaviour unless you are watching the temporary directory for files being written and quickly deleted. As far as I can tell, the only difference is that it works even if there is no writable temporary directory.

@mdboom

This comment has been minimized.

Show comment Hide comment
@mdboom

mdboom Mar 19, 2013

Owner

I was thinking more about a difference caused by using a stream vs. a random access file handle. (We've had similar issues with the PDF backend). But it seems that the postscript backend doesn't exhibit such problems. I think this is good to go.

Owner

mdboom commented Mar 19, 2013

I was thinking more about a difference caused by using a stream vs. a random access file handle. (We've had similar issues with the PDF backend). But it seems that the postscript backend doesn't exhibit such problems. I think this is good to go.

mdboom added a commit that referenced this pull request Mar 19, 2013

Merge pull request #1826 from mgiuca-google/backend_ps-no-temp-file
backend_ps: Do not write to a temporary file unless using an external distiller

@mdboom mdboom merged commit 4703f2f into matplotlib:master Mar 19, 2013

1 check passed

default The Travis build passed
Details

@mgiuca-google mgiuca-google deleted the mgiuca-google:backend_ps-no-temp-file branch Mar 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment