Opened 9 years ago

Closed 9 years ago

#16645 closed defect (fixed)

Restore displaying animations in notebook

Reported by: gagern Owned by:
Priority: major Milestone: sage-6.3
Component: graphics Keywords: notebook, animate
Cc: Merged in:
Authors: Martin von Gagern Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 2c5f58b (Commits, GitHub, GitLab) Commit: 2c5f58bd16352a82c7c7b9ad6f1c17cd86a33571
Dependencies: #15515, #16608 Stopgaps:

Status badges

Description

#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.

Change History (11)

comment:1 Changed 9 years ago by gagern

Branch: → u/gagern/ticket/16645
Created: Jul 11, 2014, 7:10:33 AM → Jul 11, 2014, 7:10:33 AM
Modified: Jul 11, 2014, 7:10:33 AM → Jul 11, 2014, 7:10:33 AM

comment:2 Changed 9 years ago by gagern

Commit: → 9f30e9400f6fda042bf579e720ffee772c751f1e
Status: new → needs_review

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.

comment:3 Changed 9 years ago by gagern

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.

comment:4 Changed 9 years ago by git

Commit: 9f30e9400f6fda042bf579e720ffee772c751f1e → bb7307f0649cfa90916f74c102d1d09a424b759a

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

​bb7307fPrevent doctest from leaking file into current working directory.

comment:5 Changed 9 years ago by gagern

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

comment:6 Changed 9 years ago by vbraun

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

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

in doctests.`

comment:7 Changed 9 years ago by vbraun

Reviewers: → Volker Braun

comment:8 Changed 9 years ago by vbraun

Status: needs_review → positive_review

lgtm

comment:9 Changed 9 years ago by git

Commit: bb7307f0649cfa90916f74c102d1d09a424b759a → 2c5f58bd16352a82c7c7b9ad6f1c17cd86a33571
Status: positive_review → needs_review

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.

comment:10 in reply to:  6 Changed 9 years ago by gagern

Status: needs_review → positive_review

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.

comment:11 Changed 9 years ago by vbraun

Branch: u/gagern/ticket/16645 → 2c5f58bd16352a82c7c7b9ad6f1c17cd86a33571
Resolution: → fixed
Status: positive_review → closed
Note: See TracTickets for help on using tickets.