Fix 2d animations

[vbraun]

Various things in 2d animations need refactoring

• The show_path optional argument should be removed everywhere, and meaningful return values should be implemented
• (Lack of) return values sometimes contradicts documentation, e.g. in Animation.apng()
• Functions that save should have a mandatory (positional) filename as argument
• There should be a keyword argument to switch between different output types in show(), similar to viewer='jmol' in 3d graphics.
• Make up your mind on whether Animate._frames is 2d graphics or input to plot()

sage: a = animate([sin(x + float(k)) for k in srange(0,2*pi,0.7)])
sage: a._frames
[sin(x),
 sin(x + 0.7),
 sin(x + 1.4),
 sin(x + 2.0999999999999996),
 sin(x + 2.8),
 sin(x + 3.5),
 sin(x + 4.2),
 sin(x + 4.9),
 sin(x + 5.6000000000000005)]
sage: a.graphics_array()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-19-9b519c8280fc> in <module>()
----> 1 a.graphics_array()

/home/vbraun/Code/sage.git/local/lib/python2.7/site-packages/sage/plot/animate.pyc in graphics_array(self, ncols)
    510         if rem > 0:
    511             nrows += 1
--> 512         return plot.graphics_array(frame_list, nrows,  ncols)
    513 
    514     def gif(self, delay=20, savefile=None, iterations=0, show_path=False,

/home/vbraun/Code/sage.git/local/lib/python2.7/site-packages/sage/plot/plot.pyc in graphics_array(array, n, m)
   2442         n = int(n)
   2443         m = int(m)
-> 2444         array = reshape(array, n, m)
   2445     return GraphicsArray(array)
   2446 

/home/vbraun/Code/sage.git/local/lib/python2.7/site-packages/sage/plot/plot.pyc in reshape(v, n, m)
   2360     if not isinstance(v[0], Graphics):
   2361         # a list of lists -- flatten it
-> 2362         v = sum([list(x) for x in v], [])
   2363 
   2364     # Now v should be a single list.

TypeError: 'sage.symbolic.expression.Expression' object is not iterable

Depends on #7298
Depends on #16573
Depends on #17234

CC: @gagern

Component: graphics

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

[vbraun]

comment:1

Also, PIL(low) can read/write animated gif, so get rid of the ImageMagick dependenecy

[vbraun]

Dependencies: #17234

[vbraun]

comment:3

For future reference, the IPython notebook cannot display gifs (animated or not): ipython/ipython#2115

[gagern]

Changed dependencies from #17234 to #7298, #16573, #17234

[gagern]

Replying to @vbraun:

• The show_path optional argument should be removed everywhere, and meaningful return values should be implemented

I had, at some point in the past, myself proposed to return path names from all the file creation functions. That's #16573 which I wrote at a time when animations were even more severely broken. Since noone seemed to care about it, and I was mostly happy again once notebook worked all right again, I left it at that.

• (Lack of) return values sometimes contradicts documentation, e.g. in Animation.apng()

The APNG inconsistency is in a certain sense a consequence of that: back in #16533 I had code which got refactored to #16573 about returning paths and to #16571 for APNG support. The latter went forward, the former was left to rot, and I didn't adjust the docs properly. Sorry there.

• Functions that save should have a mandatory (positional) filename as argument

Mandatory positional file name arguments would break backwards compatibility. At the moment, it is perfectly all right for a notebook user to write a.gif() resp. a.ffmpeg() in order to view an animation a in a given format. The documentation seems to encourage that behavior. So if we follow the deprecation guideline, we have to first spit out warnings for a year before we really make this mandatory. The gif method has a different order of arguments at the moment, so a positional argument there would already have a different meaning. We would have to make positional use of delay deprecated, either at the same time (using the type of the argument to distinguish between backward-compatible delay and forward-compatible file name) or sequentially. If we'd do it sequentially, i.e. first deprecate the delay (in my opinion preferrably using #16607) and then deprecate the lack of a file name, then it would take two years. Or you are of the opinion that animations are so generally broken that you don't care about graceful deprecation, and have to convince some other devs to agree to that view. I wouldn't feel comfortable giving a positive review to such a change.

• There should be a keyword argument to switch between different output types in show(), similar to viewer='jmol' in 3d graphics.

#7298 introduced a keyword argument for show which I called format. I suggest we continue discussion of format switching in that ticket.

• Make up your mind on whether Animate._frames is 2d graphics or input to plot()

I had the impression that one should be able to provide plot input as an argument to animate for the sake of simplicity, and that one should be able to provide a generator instead of a list for the sake of memory efficiency. So having _frames contain either of these, or even a mixture, seems hard to avoid at the moment. That said, there obviously should be no exceptions due to such mixtures. I'd say we introduce a method which returns an iterator over graphics objects, moving the logic from the png method into that.

I feel I could write a branch for the APNG documentation and the _frames handling which would be ready for inclusion very soon. If I include the show_path issue, I could deal with all of that in a new branch for #16573. The mandatory positional file name argument issue needs some discussion first, I think. Do you want this ticket here focused on that issue, or do you want to open a new one exclusively about that and close this here as duplicate of all of these others?

[vbraun]

comment:5

Replying to @gagern:

Mandatory positional file name arguments would break backwards compatibility.

Fixing bugs breaks backward compatibilty if you really want the bug. But that is besides the point. Silently creating files is a terrible UI and will not work with anything but SageNB. The gui isn't only SageNB any more, so everything that can't work with IPython notebook (say) is by definition a bug. You don't need to deprecate bugs for a year until you finally fix that bug.

[gagern]

comment:6

Replying to @vbraun:

Fixing bugs breaks backward compatibilty if you really want the bug. But that is besides the point. Silently creating files is a terrible UI and will not work with anything but SageNB. The gui isn't only SageNB any more, so everything that can't work with IPython notebook (say) is by definition a bug. You don't need to deprecate bugs for a year until you finally fix that bug.

On SageNB you currently have a feature (perhaps badly designed, but not a bug) which allows calling these methods with no arguments. That feature is currently documented and presumably being used. Now that feature doesn't work for IPython notebook, and instead appears to fail silently. Agreed, that's a bug. So there are two things to fix: the documentation so it clarifies the situation, and the implementation so users won't be confused.

In my opinion, the documentation should indicate that this syntax is supported on SageNB only, and is deprecated. The implementation should issue a warning about the deprecation. Both should direct users to call save or show instead. Doing all of this would in my opinion solve the bug. I see no reason which would force us to break backwards compatibility any more than this.

[vbraun]

comment:7

Replying to @gagern:

On SageNB you currently have a feature (perhaps badly designed, but not a bug) which allows calling these methods with no arguments.

Bring back the spacebar heating feature! http://xkcd.com/1172/

Seriously, just say no. There is bad design as in: bad taste, and bad design as in: thats horrible and it can't work. The latter is a bug.

[jdemeyer]

comment:8

Replying to @vbraun:

Seriously, just say no. There is bad design as in: bad taste, and bad design as in: thats horrible and it can't work. The latter is a bug.

Bad design is not the same as a bug. If it currently works in sagenb, it's a feature (despite the bad design), not a bug.

[vbraun]

comment:9

It still needs to be changed to work in the IPython notebook, so its a bug and a feature at the same time.

[kcrisman]

comment:10

See also #22777 and/or #16573.