Pluto ux process file drop

[pankgeorg]

Excel (and everything TableIO supports)

Summary of the status

• Factor out Frontend logic in hook (@dralletje review)
• Factor out backend logic in package (kudos @lungben !)
• Notify user file is being saved (me)
• Don't allow on non-empty cells with user friendly message (me)
• Allow non-empty cells drop, add cell below
• Drop on the whitespace results in a new cells at the bottom of the notebook
• Use TableIOInterface (@lungben review + code)
• Get extensions from TableIOInterface (@lungben review)
• let-based code sample blocks (@dralletje review)
• drop base64 for filesave (@dralletje review)

test26.jl.Pluto.jl.-.Google.Chrome.2020-12-30.17-40-05_Trim.mp4

[lungben]

Looks great!

In Windows the file path is generated with backslashes as directory separators, which gives the following error: syntax: invalid escape sequence. It could be fixed by making it a raw string:

begin
    using TableIO, DataFrames
    df_test = DataFrame(read_table(raw"C:\Users\xxx\.julia\pluto_notebooks\test.csv",
    #, "Sheet1"  # Uncomment This if you need to read a specific sheet
    ); copycols=false)
end

However, I would prefer to use relative paths here so that the notebook stays portable. This could be either a temporary directory or relative to the home directory (as in your implementation).

begin
    using TableIO, DataFrames
    df_test = DataFrame(read_table(joinpath(homedir(), ".julia/pluto_notebooks"),
    #, "Sheet1"  # Uncomment This if you need to read a specific sheet
    ); copycols=false)
end

The line
#, "Sheet1" # Uncomment This if you need to read a specific sheet
should only be shown when an Excel file is imported (currently only xlsx is supported by TableIO, but I could add at least xlsm (files with Macros)).
For CSV files in Windows, the MIME type may be set to Excel, therefore it is imho safer to rely on the file extension here.

You could also support SQLite files (with extensions sqlite, sqlite3 or db), then adding the table name as 2nd argument to read_table (which needs to be filled in by the user).

Maybe you can re-use the file extensions defined in TableIO here?
TableIO.FILE_EXTENSIONS, _get_file_type(filename) - I could add documentation and rename the latter to make them a public interface.

[pankgeorg]

Thank you so much @lungben !
Yes I will add support for everything TablesIO can read.

I've noticed the CSV mime type, and the code handles that as Excel, but the TableIO then handles correctly (kudos for that!)

I also agree with the relative paths:

Between

1. .assets folder relative to notebook (that should be moved when you move the notebook file)
2. ~/.pluto/assets directory in home

what do you think is best?

[lungben]

I am not sure how the MIME types are set, by the browser? Excel associates itself per default with the csv extension, this gives (at least on my machine) the MIME type "application/vnd.ms-excel" for csv files. For "sqlite" files no MIME type is set at all for me. Thus I think it is safer to rely on the file extensions than on the MIME type (although not perfect).

For the storage of the imported files, I think they should be next to the notebook file, i.e. option 1. The folder should be unique per notebook so that multiple notebook.jl files in the same directory are supported, each with its own assets folder.

Suggestion:
my_notebook.jl -> my_notebook.assets
where my_notebook.assets could be a folder or a zip archive.

Not sure about @fonsp plans for the package environments (#142 ), but such an assets folder could also be used to store Project.toml and Manifect.toml files.

Edit:
With TableIO, you could also directly load files inside a zip file:
read_table("zip_file.zip", "data.csv")
This should work for all supported types, except JDF (which is a directory structure, not a single file).

[pankgeorg]

Summary of the status

• Factor out Frontend logic in hook (@dralletje review)
• Factor out backend logic in package (kudos @lungben !)
• Notify user file is being saved (me)
• Don't allow on non-empty cells with user friendly message (me)
• Use TableIOInterface (@lungben review + code)
• Get extensions from TableIOInterface (@lungben review)
• let-based code sample blocks (@dralletje review)
• drop base64 for filesave (@dralletje review)

[lungben]

I registered TableIOInterface yesterday evening, it should be available in the Registry in 3 days.
I'll add a function ˋis_extension_supported(extension)ˋ to the package probably today afternoon.

Update: the package is registered.

[lungben]

Looks great!

2 minor points:

1. Could you please rebase on master? It would be good to check if this PR works correctly with the most recent Pluto changes, especially with the automatic function wrapping.
2. When pulling a file on an empty cell, this cell is shortly marked with a non-empty cell warning before upload of the file is finished.

[pankgeorg]

Looks great!

2 minor points:

1. Could you please rebase on master? It would be good to check if this PR works correctly with the most recent Pluto changes, especially with the automatic function wrapping.
2. When pulling a file on an empty cell, this cell is shortly marked with a non-empty cell warning before upload of the file is finished.

1. I rebased to master - can't say I understand if it works or breaks (Seems to work)
2. Silly me! Fixed! Thanks for catching!!

[pankgeorg]

Update: A function-like test seems to break

[lungben]

Cells which contain import are not wrapped into a function call, see

https://github.com/fonsp/Pluto.jl/pull/720/files#diff-04c7252262a0e6de2db2cded7aba9b1130c1e5ba6adff25bf8ec3a34e36a4788R928-R946

Therefore, in principle at least the TableIO templates should still work.

[dralletje]

Good job on putting the javascript in its own file

[lungben]

Works perfectly for me, thanks!!!

[lungben]

One thing is missing: if the notebook is renamed / moved using the Pluto functionality, the .assets directory should be moved, too.

I am currently adding support of this into TableIOInterface so that the generated code does not need to be modified when the notebook is moved.
Then Pluto needs only to take care of the moving of the directory itself.

[pankgeorg]

Very good point Benjamin; I'll try to fix this - I think I know how

[lungben]

One more small thing:
when moving a Pluto cell with the mouse, the new error message "This cell is not empty" is shown for the adjacent cells.
You can still move a cell when you are careful to put your cursor exactly in the middle between 2 cells, but this is a bit tricky.

[fonsp]

Some comments:

• Use PlutoUI.LocalResource for images, audio & video instead of Images+ImageMagick
• The red aggressive scary warning is something that we should solve, not the user
• Pluto has a function numbered_until_unique that we can use for the local filename, I don't think it's a feature that old files with a clashing name are overwritten

What do you think?

[lungben]

Maybe to make it really fancy:
if you drag a file to a cell where a file with the same filename is already imported, we could ask whether to replace the existing file or to add it as a new file. This could be useful when updating or correcting input data.
But this is rather a nice-to-have feature for me.

[pankgeorg]

-> Added 4 lines that move the assets folder along with the notebook!

Benjamin, I'll fix the message-hover issue too.
Fons, got the message for the drag & drop message too :) - I'll try to add the blue message and create a new cell (and remove the red message completely).

Also, LocalResource it is!

Also, files that don't have special code will get copied and we'll add a filename = "path_to_file" string so the user know what to do next.

I'll also try the numbered_until_unique for extra drops of the same file

[lungben]

As discussed, please use this format so that it still works after moving / renaming the notebook.

joinpath(split(@__FILE__, '#')[1] * ".assets", "my_file.jpg")

[pankgeorg]

Note to self: make sure the name change the dir name changes too (maybe that happens already)

[fonsp]

oops sorry

[lungben]

Is anything still missing here?
It would be great to have this feature merged.

[fonsp]

Can you add a [deps] entry for tableiointerface?

[pankgeorg]

#950
more features