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

Graphics3d saves every png image twice #15728

Closed
nilesjohnson mannequin opened this issue Jan 24, 2014 · 11 comments
Closed

Graphics3d saves every png image twice #15728

nilesjohnson mannequin opened this issue Jan 24, 2014 · 11 comments

Comments

@nilesjohnson
Copy link
Mannequin

nilesjohnson mannequin commented Jan 24, 2014

In Graphics3d.save sage creates a png file and then uses PIL to convert it to whatever format was requested. But the check for whether to save another format is broken and so sage always creates a second file, even if it's just converting from png to png.

The reason for this is a broken check for the file extension:

if ext != 'png' should instead be if ext != '.png' since ext begins with a dot!

CC: @nilesjohnson

Component: graphics

Author: Philip Robinson

Branch/Commit: u/niles/ticket/15728 @ d6024fe

Reviewer: Niles Johnson

Issue created by migration from https://trac.sagemath.org/ticket/15728

@nilesjohnson nilesjohnson mannequin added this to the sage-6.1 milestone Jan 24, 2014
@nilesjohnson nilesjohnson mannequin added c: graphics labels Jan 24, 2014
@sagetrac-probinso
Copy link
Mannequin

sagetrac-probinso mannequin commented Jan 29, 2014

comment:2

(new to system, so please forgive)

on line 1290 :

if ext == 'png'

on line 1298 :

if ext != 'png'

so both instances of strcmp parameters are ill formed. I would like to change both of these in my commit.

@sagetrac-probinso sagetrac-probinso mannequin self-assigned this Jan 29, 2014
@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Jan 29, 2014

comment:4

Replying to @sagetrac-probinso:

so both instances of strcmp parameters are ill formed. I would like to change both of these in my commit.

Right -- both should be changed :) When you upload the new commit, I'll be happy to review it.

@sagetrac-probinso
Copy link
Mannequin

sagetrac-probinso mannequin commented Jan 29, 2014

Branch: u/probinso/ticket/15728

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

Commit: d6024fe

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

Changed branch from u/probinso/ticket/15728 to u/niles/ticket/15728

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

comment:7

Sorry I dropped this for a while -- rebased to latest development branch (6.2.beta0) and looking at it now.


New commits:

4b87c01Fixed file extension checking saving for plot3d, to insure excess files aren't created
d6024feTrac #15728: rebase to 6.2.beta0

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

Author: Philip Robinson

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

Reviewer: Niles Johnson

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

comment:8

I'm happy with the changes and all tests pass; positive review!

@sagetrac-probinso
Copy link
Mannequin

sagetrac-probinso mannequin commented Feb 7, 2014

comment:10

Thank you, for reviewing now that I have completed the process once I feel better about moving forward. I will remember to flag later bugs as 'needs_review' next time.

@nilesjohnson
Copy link
Mannequin Author

nilesjohnson mannequin commented Feb 7, 2014

comment:11

No worries -- good job :) Also remember to include your full name in the Authors section of the ticket properties.

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

No branches or pull requests

1 participant