Ticket #12827 (needs_work enhancement)
Expand Animation class to accept more graphics types
| Reported by: | niles | Owned by: | jason, was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.10 |
| Component: | graphics | Keywords: | animate, graphics, 3D |
| Cc: | gutow, nbruin, novoselt | Work issues: | docstrings, testing, think about img protocol |
| Report Upstream: | N/A | Reviewers: | |
| Authors: | Niles Johnson | Merged in: | |
| Dependencies: | Stopgaps: |
Description (last modified by niles) (diff)
Currently the Animation class only animates Graphics objects, and throws an error for Graphics3d, GraphicsArray, and Tachyon objects. The behavior of this class should be improved to accept any Sage object whose save method produces an image.
See this sage-devel thread for the beginning of this discussion, including some discussion of possible solutions.
Apply
Attachments
Change History
Changed 14 months ago by niles
-
attachment
trac_12827_animate3d.patch
added
comment:1 Changed 14 months ago by niles
- Cc gutow, nbruin added
A first patch is up now, which makes the type-checking of the Animate class more inclusive. However I am now leaning toward a different idea: adding an img() method to each of the major graphics classes defined as follows:
def img(self, *args, **kwds):
return self.save(*args, **kwds)
Then for the Animation class, or other situations where an image representation of an object is required, one would call the img() method, and reject the object if it has no such method. To handle new object types, one would implement an img() method for those types. As I understand them, this is more closely aligned with the principles of object-oriented programming, and thus more appropriate for Sage library code.
I would appreciate hearing further thoughts on the relative merits of the current patch v.s. the img() method idea, as well as thoughts on naming this method (other suggestions are save_as_bitmap() and render_image()). Maybe the name should be prefixed or suffixed with an underscore too?
Changed 14 months ago by niles
-
attachment
trac_12827_animate3d_img_method.patch
added
add img method to the four major graphics classes
comment:2 Changed 14 months ago by niles
- Work issues set to docstrings, testing, think about img protocol
- Authors set to Niles Johnson
The second patch is a replacement for the first, using the idea of an img() method instead of type-checking. All tests pass, but I haven't finished writing docstrings.
Patchbot: apply trac_12827_animate3d_img_method.patch
comment:3 follow-up: ↓ 4 Changed 14 months ago by nbruin
OK, I'm afraid the following if a bit of distraction of your core goal. Without reading any documentation, I would expect G.img() to return an "image object", rather than the G.save_as_bitmap("filename"). Of course save_as_bitmap still needs to be told in what format the bitmap should be saved.
I'm not sure we have an appropriate "image object". We do have PIL in sage. Try import Image. Would it make sense to interface with their types for an "img" return value?
Obviously, having a "convert to bitmap object" protocol implemented for all graphics-type objects, together with a save-to-file option on the bitmap object, would give you the required infrastructure to produce animations. It may not be the most efficient (or even sufficiently efficient) way of doing it, though ...
I can understand if you don't want to mess with bitmap objects (whatever they should be) and just settle for a way to dump a bitmap representation of objects into a file. However, you might not want to use "img" then, reserving that word for when someone does want to do that.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 14 months ago by niles
Replying to nbruin:
Without reading any documentation, I would expect G.img() to return an "image object"
Hmm, yes I see your point. Thanks for the tip about PIL -- that package does seem to provide a pretty good Image object, which is created by reading an image. So one might do something like the following:
def img(self, filename, *args, **kwds):
self.save(filename=filename, *args, **kwds)
from PIL import Image
im = Image.open(filename)
return im
Maybe this is the wrong way to go about it though, since the save() and show() methods of the resulting Image object would re-save the image to a file, wasting a lot of time. Image also has the capability to read an image from a stream or a buffer, but I don't know how to create such things from Sage's graphics objects.
On the other hand, the current show() method of Sage's graphics objects saves the image to a file before showing it, so this implementation would be following that precedent.
A more rudimentary compromise might be to have a method which saves the image and returns the system path to the image:
def img(self, filename, *args, **kwds):
self.save(filename=filename, *args, **kwds)
return filename
At this stage, I've given three different possible img methods, and I can see now how each one establishes a different protocol. Since this ticket is probably not the right place to try to establish a protocol, I think I'll try to use a different name. I'm really having a hard time warming up to the term "bitmap" though. Wikipedia explains to me that it is essentially correct, although the image types we would use by default, like JPG, PNG, TIFF, are compressed bitmap images. What do you think of save_image instead?
comment:5 in reply to: ↑ 4 Changed 14 months ago by nbruin
Replying to niles:
... Maybe this is the wrong way to go about it though, since the save() and show() methods of the resulting Image object would re-save the image to a file, wasting a lot of time. Image also has the capability to read an image from a stream or a buffer, but I don't know how to create such things from Sage's graphics objects.
Well, in some cases this would be the best thing we have available. In the case of 3D, for instance, the rendering is done with the external program tachyon, which get a scene description in a file and saves a bitmap into another file. So in that case, going through an Image object for animate is very wasteful.
Googling shows that matplotlib can be used to produce the bitmap in-memory and directly feed into Image. Incidentally, if those files are written in /tmp (or any tmpfs filesystem), the files likely never hit disk. They'll just live in memory-based buffers.
A more rudimentary compromise might be to have a method which saves the image and returns the system path to the image:
def img(self, filename, *args, **kwds): self.save(filename=filename, *args, **kwds) return filename
I personally like the rule that routines used for their side-effect do not return a value. Especially when you have to specify the filename anyway, it doesn't really make sense to return it. A "success" return value could work, but raising an exception on failure is more pythonic.
At this stage, I've given three different possible img methods, and I can see now how each one establishes a different protocol. Since this ticket is probably not the right place to try to establish a protocol, I think I'll try to use a different name. I'm really having a hard time warming up to the term "bitmap" though. Wikipedia explains to me that it is essentially correct, although the image types we would use by default, like JPG, PNG, TIFF, are compressed bitmap images. What do you think of save_image instead?
I think it's an improvement over "save", because to me "save" indicates a reversible operation (via "load"). In the case here, the bitmap is a different kind of object from which the original object cannot be reconstructed. I think save_as_image is even clearer, but it's also longer and has more underscores.
In general, people would differentiate "bitmap" and "vector graphics" formats inside "image" formats (i.e., jpg, png versus pdf, ps, svg, disregarding that the latter can also contain bitmap information). On the application side, GIMP versus Inkscape or Photoshop versus Acrobat/Illustrator?.
For 3D scenes we currently have no way of creating vector graphics representations, although those would be quite useful for producing print-quality schematics. matplotlib can create excellent vector graphics as well as bitmaps.
If save_as_image supports a format specifier (and/or derives a default from inspecting the filename for known extensions) that also supports "eps, svg, inkscape" then I think it's a very good name. Not all object have to support all formats of course. If you are going to categorically restrict to bitmap formats (compressed or not), I think sage_as_bitmap is better.
Look at how matplotlib solves this problem. Staying compatible with them
comment:6 Changed 13 months ago by gutow
My take on this is that G.image() should return an image object. I haven't looked at PIL, but I would expect an image object to have a publicly accessible .type value that tells the image type and a .data or .imgdata that contains the binary representation of the image. I would suggest that we want graphics objects to have two routines G.image(type =XXX, height=XXX, width=XXX, kwds) and G.save_image(type=XXX, filename=XXX, height=XXX, width=XXX, kwds). I suppose the height and width could have default values. G.save_image should return a success value, but raise an exception on failure.
Changed 13 months ago by niles
-
attachment
trac_12827_animate3d_save_image.patch
added
a rudimentary save_image method
comment:7 Changed 13 months ago by niles
- Status changed from new to needs_review
- Description modified (diff)
Thanks for all the feedback. Having thought through the issues here, I'd like to provide a minimal solution to the animation problem now which doesn't conflict with potential future improvements to the graphics objects. I've opted for a save_image method which just passes its arguments to the save method. This can save in bitmap or vector formats, depending on the capabilities of the save method, and sizing of the output image will depend on the save method too.
All tests pass in the affected files, and all of the documentation builds without warning; Nils or Jonathan, are either of you interested in reviewing this?
Patchbot: apply trac_12827_animate3d_save_image.patch
comment:9 Changed 12 months ago by kcrisman
Perhaps one should even just accept graphics files like pngs? Just a thought.
comment:10 Changed 12 months ago by kcrisman
- Status changed from needs_review to needs_work
Sadly, this doesn't apply to Sage 5.0 - probably because of the refactoring of the graphics code to be largely in sage/plot/graphics.py. But I really like this potential! See for instance this ask.sagemath.org question.

uses more robust type-checking for inputs