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

Restore displaying animations in notebook #16645

Closed
gagern mannequin opened this issue Jul 11, 2014 · 14 comments
Closed

Restore displaying animations in notebook #16645

gagern mannequin opened this issue Jul 11, 2014 · 14 comments

Comments

@gagern
Copy link
Mannequin

gagern mannequin commented Jul 11, 2014

#12827 broke display of animations, and while #16608 restored that functionality for Animation.show(), it remains broken for methods Animation.gif() and Animation.ffmpeg(). Based on #15515, a more complete fix is possible.

Depends on #15515
Depends on #16608

Component: graphics

Keywords: notebook, animate

Author: Martin von Gagern

Branch/Commit: 2c5f58b

Reviewer: Volker Braun

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

@gagern gagern mannequin added this to the sage-6.3 milestone Jul 11, 2014
@gagern gagern mannequin added c: graphics labels Jul 11, 2014
@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 11, 2014

Branch: u/gagern/ticket/16645

@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 11, 2014

Commit: 9f30e94

@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 11, 2014

New commits:

58b9c46minimal fix to restore functionality
16cbd2dMerge branch 'develop' into ticket/16533-stopgap
b3e656bgraphics_filename: return a tmp_filename() if not in EMBEDDED_MODE
69b285dRestore possibly positional arguments.
e3c5542Merge branch ticket/15515 into ticket/16608 to create ticket/16645.
9f30e94Always use graphics_filename instead of tmp_filename.

@gagern gagern mannequin added the s: needs review label Jul 11, 2014
@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 11, 2014

comment:3

Note that only 9f30e94 is new for this ticket here, all the other commits listed above are parts of some already closed tickets and should end up in the develop branch pretty soon. So it is enough to review just that single commit.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 11, 2014

Changed commit from 9f30e94 to bb7307f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 11, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

bb7307fPrevent doctest from leaking file into current working directory.

@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 11, 2014

comment:5

These are the changes that need reviewing in this ticket here.

@vbraun
Copy link
Member

vbraun commented Jul 13, 2014

comment:6

tmp_filename is already a global in Sage, you don't need

sage: f = sage.misc.temporary_file.tmp_filename(ext='.png')

in doctests.`

@vbraun
Copy link
Member

vbraun commented Jul 13, 2014

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Jul 13, 2014

comment:8

lgtm

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2014

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

2c5f58bAvoid unneccessary qualification of tmp_filename in doctests.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2014

Changed commit from bb7307f to 2c5f58b

@gagern
Copy link
Mannequin Author

gagern mannequin commented Jul 13, 2014

comment:10

Replying to @vbraun:

tmp_filename is already a global in Sage […]

Thanks for the review, and for pointing this out. I did copy and paste that from 745a3bb from #12827, and addressed both instances now.

@vbraun
Copy link
Member

vbraun commented Jul 14, 2014

Changed branch from u/gagern/ticket/16645 to 2c5f58b

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