Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper functions #144

Merged
merged 12 commits into from
Nov 10, 2021
Merged

Helper functions #144

merged 12 commits into from
Nov 10, 2021

Conversation

waralex
Copy link
Collaborator

@waralex waralex commented Nov 2, 2021

Implementation of functions for working with the dcc_download component and an analogue of Python's Format to help in formatting the DataTable

@waralex waralex requested a review from alexcjohnson November 2, 2021 18:14
@github-actions github-actions bot added enhancement New feature or request tests labels Nov 2, 2021
@waralex waralex linked an issue Nov 3, 2021 that may be closed by this pull request
callback!(app, Output("download", "data"), Input("download-btn", "n_clicks"), prevent_initial_call = true) do n_clicks
return dcc_send_bytes("df.arr") do io
Arrow.write(io, df)
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This form where the first arg to dcc_send_bytes is a function - it's a bit more cumbersome and confusing than the Python version that looks like

@app.callback(Output("download", "data"), Input("btn", "n_clicks"))
def func(n_clicks):
    return dcx.send_data_frame(df.to_csv, "mydf.csv")

The Python one is already a little unintuitive - I wish pandas would have just allowed df.to_csv to output a string if called with no buffer or file path - but at least you don't have to explicitly create a new function referencing an io that you don't care about. I think if I were using this I'd prefer to just write it out myself:

callback!(app, Output("download", "data"), Input("download-btn", "n_clicks"), prevent_initial_call = true) do n_clicks
    io = IOBuffer()
    Arrow.write(io, df)
    return dcc_send_bytes(take!(io), "df.arr")

More characters but the same number of lines in total, and less to learn about this interface. Is there any other way we can make this cleaner? Are there other use cases where the function to be called as src(io) is already defined, the way df.to_csv is? If not I might suggest dropping the src::Function form.

Copy link
Collaborator Author

@waralex waralex Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dcc_send_bytes(take!(io), "df.arr") is also already possible. (dcc_send_bytes(src::AbstractVector{UInt8}, filename; type = nothing))
The version with callback and io seemed convenient to me. However, if you think that it is confusing, then I can remove it

Copy link
Collaborator Author

@waralex waralex Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not make an analogue of send_data_frame because this would lead to a direct dependence of Dash on DataFrames and all packages of a particular serialization (CSV, Arrow, etc.) and thus would mean the need for a release every time the version of these packages changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at 925744e i change dcc_send_bytes(src::Function, filename; type = nothing) to dcc_send_bytes(writer::Function, data, filename; type = nothing) so example above is now look like

return dcc_send_bytes(Arrow.write, df, "df.arr")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner! 🎉

)
return JSON3.write(
(
locale = deepcopy(f.locale),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious why deepcopy is needed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are absolutely right, it is superfluous here. Removed in cc7ebaf

Copy link
Contributor

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 💃

@waralex waralex merged commit a11022c into dev Nov 10, 2021
@etpinard etpinard deleted the helper_functions branch June 13, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support dcc.send_file and dcc.send_data_frame
2 participants