save() should download files in the notebook

[vbraun]

In a browser-based notebook an object's save() method should automatically download the saved file and not just let it linger on the remote server.

I'm proposing to do this by returning, from save(), a SavedFile object instead of nothing. The SavedFile._rich_repr_ method then returns a custom rich representation which gives browser-based display backends a hook to do something with the saved file. Also, nothing happens if you call save() from within code---only if it hits the displayhook:

sage: my_plot.save()        # yes, download
sage: _ = my_plot.save()    # no automatic download
sage: for i in range(10):
....:     my_plot.save()    # depends
sage: for i in range(10):
....:     my_plot.save()    # no
....:     x = x + i

CC: @gagern @kcrisman

Component: user interface

Author: Volker Braun

Branch/Commit: u/vbraun/save_should_download_files @ b3a066a

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

[vbraun]

comment:1

The SavedFile object could itself have a show() method, which may or may not show the file (depending on the display backend capabilities and the saved file):

sage: my_plot.save(**complicated_options).show()
The Foo Notebook cannot display $complicated_graphics_format

[jdemeyer]

Replying to @vbraun:

In a browser-based notebook an object's save() method should automatically download the saved file

I'm not sure that I agree with this. You might want to save() the object on the server to load() it in some Sage session later.

If you want download functionality, I would prefer to use a new .download() method for that.

[vbraun]

comment:3

Just to clarify, save() should of course still save the file on the remote file system. It just should in addition get the file to the client.

Tying it to the displayhook already gives you a means to control whether or not the file is downloaded. Also, if you really just save a temporary file on the server you are more likely to do that form within code so it wouldn't be downloaded anyway. There could be other switches, maybe foo.save(download=False) or %download [on|off]. But just foo.save() should by default do something sensible. If you don't like the default you can then look into the docs. But if you only notice that save() doesn't result in a file on your laptop then the user is going to assume that this is the way it is.

[jdemeyer]

comment:4

With cloud applications (let's consider sagenb as such an application), "save" usually means "save on the remote system". The notebook interface has "Save" and "Save & quit" buttons, neither of these result in a downloaded file.

[vbraun]

comment:5

From a user these are two different activities:

• checkpointing the current worksheet

• creating a media file to be used elsewhere
  I agree that its confusing that both are called "save", and it all goes back to the time when Word would crash every 30 minutes so you better save your document to three different floppies. Now we still call it save and we often even have a little picture of a floppy disc under it (anyone remember these?), but its really just a pragmatic idiom for checkpointing nowadays (see also https://github.com/ipython/ipython/wiki/IPEP-15:-Autosaving-the-IPython-Notebook).

[vbraun]

Branch: u/vbraun/save_should_download_files

[vbraun]

Commit: 91830de

[vbraun]

comment:7

Apparently not so easy in IPython (ipython/ipython#8053), I implemented a moderately hackish workaround.

[sagetrac-git]

Changed commit from 91830de to 16cb948

[sagetrac-git]

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

16cb948

Add SageNB support, automatic download in the ipython notebook

[vbraun]

Author: Volker Braun

[vbraun]

comment:9

SageNB often replaces the cell dom with the saved part even for cells that you don't currently work on. So javascript inside the cell can't really initiate the download or it would do it again all the time. So its just a download link in SageNB.

[sagetrac-git]

Changed commit from 16cb948 to e9ad72a

[sagetrac-git]

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

e9ad72a

fix doctests

[sagetrac-git]

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

abaf94b

Merge branch 'develop' into #17942

[sagetrac-git]

Changed commit from e9ad72a to abaf94b

[vbraun]

comment:12

This works now in both notebooks, imho it is the cleanest way of expressing what you want to do:

plot(f).save('/tmp/foo.png').show()

[sagetrac-git]

Changed commit from abaf94b to 18968d0

[sagetrac-git]

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

18968d0

Merge Sage-6.7.beta2 into #17942

[sagetrac-git]

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

c9f1718

fix documentation build

[sagetrac-git]

Changed commit from 18968d0 to c9f1718

[sagetrac-git]

Changed commit from c9f1718 to ed9898d

[sagetrac-git]

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

ed9898d

documentation fix

[sagetrac-git]

Changed commit from ed9898d to 43a6b48

[sagetrac-git]

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

43a6b48

Fix doctests

[fchapoton]

comment:17

does not apply, needs rebase

[sagetrac-git]

Changed commit from 43a6b48 to d8b4b81

[sagetrac-git]

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

d8b4b81

Merge Sage-6.9.beta2 into t/17942/save_should_download_files

[vbraun]

comment:20

Fixed

[fchapoton]

comment:21

does not apply

[sagetrac-git]

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

b3a066a

Merge 6.10.beta3 into Trac #17942

[sagetrac-git]

Changed commit from d8b4b81 to b3a066a

[kcrisman]

comment:24

This looks pretty robust. Should we expect any other .save() methods to implement this SavedFile type? Such as the usual .sobj saving... I assume not, but just checking.

Also, this doesn't appear to require any changes in sagenb; I ask because I'm looking at that right now, but I don't see anything that does.

[vbraun]

comment:25

Replying to @kcrisman:

This looks pretty robust. Should we expect any other .save() methods to implement this SavedFile type? Such as the usual .sobj saving... I assume not, but just checking.

I don't really have an opinion either way; We could.

Also, this doesn't appear to require any changes in sagenb; I ask because I'm looking at that right now, but I don't see anything that does.

No changes to SageNB required.

[kcrisman]

comment:26

Great, I didn't think so. I guess as long as this does what we intend for now (does it make a link for non-graphics files too? just curious) then this would be a good addition.

[vbraun]

comment:27

It always makes a link regardless of the file type

[fchapoton]

comment:28

does not apply