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

Refactor Dash.jl to support same parameters as Dash for Python & R #4

Closed
7 tasks done
rpkyle opened this issue Apr 3, 2020 · 10 comments · Fixed by #18
Closed
7 tasks done

Refactor Dash.jl to support same parameters as Dash for Python & R #4

rpkyle opened this issue Apr 3, 2020 · 10 comments · Fixed by #18
Assignees

Comments

@rpkyle
Copy link
Contributor

rpkyle commented Apr 3, 2020

The current definition of the DashApp struct should be made comparable to the Dash class in R and Python (it can remain a struct, but the interface provided to users should be more or less identical):

Dash.jl/src/Dash.jl

Lines 98 to 119 in 2707170

struct DashApp
name ::String
layout ::Component
callbacks ::Dict{Symbol, Callback}
external_stylesheets ::Vector{String}
external_scripts ::Vector{String}
url_base_pathname ::String
assets_folder ::String
callable_components ::Dict{Symbol, Component}
type_links ::Dict{Symbol, Dict{Symbol, Type}}
function DashApp(name::String, layout::Component;
external_stylesheets ::Vector{String} = Vector{String}(),
external_scripts ::Vector{String} = Vector{String}(),
url_base_pathname="/",
assets_folder::String = "assets")
new(name, layout, Dict{Symbol, Callback}(),
external_stylesheets, external_scripts,
url_base_pathname, assets_folder,
Components.collect_with_ids(layout), Dict{Symbol, Dict{Symbol, Type}}()
)
end
end

For parity, if possible, we should use similar names across all backends

  • Rename callbacks to callback

The following initial additions to the Dash.jl API are proposed:

  • Add serve_locally parameter
  • Add suppress_callback_exceptions parameter
  • Add assets_ignore parameter
  • Add assets_url_path parameter
  • Add layout_get method

We should consider removing the following elements:
- [ ] Remove callable_components, any required logic can be rolled into internal component handling, in similar fashion to Python and R
- [ ] Remove type_links

  • Deprecate and eventually remove name with a warning message as in Dash for R, and suggest using title instead.

A full list of current Dash parameters appears below; eventually all three backends should support these.


🏁 Pull request Element Python R Julia
✔️ name ✔️ ✔️ ✔️
✔️ assets_folder ✔️ ✔️ ✔️
✔️ url_base_pathname ✔️ ✔️ ✔️
✔️ external_scripts ✔️ ✔️ ✔️
✔️ external_stylesheets ✔️ ✔️ ✔️
✔️ assets_url_path ✔️ ✔️ ✔️
✔️ meta_tags ✔️ ✔️ ✔️
✔️ requests_pathname_prefix ✔️ ✔️ ✔️
✔️ routes_pathname_prefix ✔️ ✔️ ✔️
✔️ suppress_callback_exceptions ✔️ ✔️ ✔️
✔️ index_string ✔️ ✔️ ✔️
✔️ show_undo_redo ✔️ ✔️ ✔️
✔️ compress ✔️ ✔️ ✔️
✔️ assets_ignore ✔️ ✔️ ✔️
✔️ serve_locally ✔️ ✔️ ✔️
✔️ eager_loading ✔️ ✔️ ✔️
include_assets_files ✔️ ✔️
assets_external_path ✔️
@waralex
Copy link
Collaborator

waralex commented Apr 3, 2020

About renaming.
In Julia struct can't have same name as a module (or package). That's why, for example, package DataFrames with main structure DataFrame. We can rename DashApp to Dash if we rename package to something else. There is another way. We can keep DashApp, but use the dash function with all the specified parameters to create It. There are no classes in Julia, so in any case, the constructor is nothing more than a factory for creating structures.

@rpkyle
Copy link
Contributor Author

rpkyle commented Apr 3, 2020

About renaming.
In Julia struct can't have same name as a module (or package). That's why, for example, package DataFrames with main structure DataFrame. We can rename DashApp to Dash if we rename package to something else. There is another way. We can keep DashApp, but use the dash function with all the specified parameters to create It. There are no classes in Julia, so in any case, the constructor is nothing more than a factory for creating structures.

Thanks @waralex, I had a feeling there was a good reason for this change. This is less of a critical difference, so it's fine to leave it as is for now. The parameters for the struct and their names are much more important, since our eventual goal is for parity across all implementations (within reason).

@waralex
Copy link
Collaborator

waralex commented Apr 4, 2020

Remove callable_components, any required logic can be rolled into internal component handling, in similar fashion to Python and R

This means giving the existence check of the component to the JS front end and checking only the validity of the name on the backend?

@waralex
Copy link
Collaborator

waralex commented Apr 4, 2020

We should consider removing the following elements:

It looks like we're having a misunderstanding because you're trying to think of Julia in terms of "Java style OOP". I also had similar problems, but apparently it was easier for me thanks to my experience in c++, which uses several paradigms.

Julia doesn't have classes in the sense of encapsulating data with methods. There are no private properties and no any other "internals". Julia has the same ideology as the standard C++ library - data should be separated from algorithms. The interface in Julia is not about what properties a structure contains, but what actions you can do with it.
For example Dictionary from Julia source code:

mutable struct Dict{K,V} <: AbstractDict{K,V}
    slots::Array{UInt8,1}
    keys::Array{K,1}
    vals::Array{V,1}
    ndel::Int
    count::Int
    age::UInt
    idxfloor::Int  # an index <= the indices of all used slots
    maxprobe::Int

    function Dict{K,V}() where V where K
        n = 16
        new(zeros(UInt8,n), Vector{K}(undef, n), Vector{V}(undef, n), 0, 0, 0, 1, 0)
    end
    function Dict{K,V}(d::Dict{K,V}) where V where K
        new(copy(d.slots), copy(d.keys), copy(d.vals), d.ndel, d.count, d.age,
            d.idxfloor, d.maxprobe)
    end
    function Dict{K, V}(slots, keys, vals, ndel, count, age, idxfloor, maxprobe) where {K, V}
        new(slots, keys, vals, ndel, count, age, idxfloor, maxprobe)
    end
end

No one treats idxfloor or maxprobe as a part of the "user interface". The user interface is a functions that works with dictionaries - get(dict, key), set(dict, key, value), haskey(dict, key) and etc.
The huge advantage of this approach is that you can write algorithms in terms of "the entity that haskey exists for" without worrying about whether it's a dictionary, a set, or something else.
So I don't see much point in removing anything from DashApp. I think it makes sense to match the dash () signature to the Python constructor.

@rpkyle
Copy link
Contributor Author

rpkyle commented Apr 4, 2020

It looks like we're having a misunderstanding because you're trying to think of Julia in terms of "Java style OOP". I also had similar problems, but apparently it was easier for me thanks to my experience in c++, which uses several paradigms.

Julia doesn't have classes in the sense of encapsulating data with methods. There are no private properties and no any other "internals". Julia has the same ideology as the standard C++ library - data should be separated from algorithms. The interface in Julia is not about what properties a structure contains, but what actions you can do with it.

That's probably true to a large extent, since I've never programmed in C++, but I have used Java. I agree that the implementation should make sense from the perspective of the language as well.

At the same time, I'm sensitive to the fact that this can result in some tension with respect to maintaining multiple backends, and the features and interfaces they provide. It certainly makes offering the framework to users of multiple languages a bit more interesting 🙂

@rpkyle
Copy link
Contributor Author

rpkyle commented Apr 4, 2020

Remove callable_components, any required logic can be rolled into internal component handling, in similar fashion to Python and R

This means giving the existence check of the component to the JS front end and checking only the validity of the name on the backend?

I think this is a minor misunderstanding -- the struct definition is a bit different from the Dash class in R or the implementation in Python.

I am looking to better understand the logic of the current implementation, I'm not suggesting that this be handled by dash-renderer. To me, callable_components appears to be exposed to the user:

Dash.jl/src/app.jl

Lines 42 to 50 in 66fa0d3

struct DashApp
name ::String
layout ::Layout
callbacks ::Dict{Symbol, Callback}
external_stylesheets ::Vector{String}
external_scripts ::Vector{String}
url_base_pathname ::String
assets_folder ::String
callable_components ::Dict{Symbol, Component}

In Python, we currently do the following:

    @layout.setter
    def layout(self, value):
        if not isinstance(value, Component) and not isinstance(
            value, _patch_collections_abc("Callable")
        ):
            raise exceptions.NoLayoutException(
                "Layout must be a dash component "
                "or a function that returns "
                "a dash component."
            )

        self._cached_layout = None
        self._layout = value

Are these unrelated? Apologies in advance if I've missed something here.

@waralex
Copy link
Collaborator

waralex commented Apr 5, 2020

callable_components is the flat dictionary of components, which can act as inputs, states or outputs, that is components for which the id is set. This dictionary is created once from the layout hierarchy (on layout! call) and is used for checking arguments passed to callback!. It is not part of the user interface, nor is the entire DashApp structure. I don't even export DashApp from the module.
From the user's point of view, there is a dash function that creates an application and a set of functions like callback! or run_server that work with this application.
The "user's point of view" is determined primarily by the documentation :) There is no way to prevent the user from using the property, or hide the property from the user. It's unusual, but it's typical for Julia

@rpkyle
Copy link
Contributor Author

rpkyle commented Apr 5, 2020

This seems reasonable -- I think a good rule of thumb for now is that anything not exposed to the user can remain as-is, but the user-facing interface and parameters should be largely the same -- while remaining idiomatic for Julia, as you've helpfully pointed out.

The good news is that we can still make Dash.jl like Dash without trying to make Julia feel like Python 🙂

I've gone ahead and crossed off the above items that don't make sense in light of our recent discussions. Thanks for taking the time to explain in more detail.

@rpkyle rpkyle changed the title Refactor Dash struct to support same parameters as Dash for Python & R Refactor Dash.jl to support same parameters as Dash for Python & R Apr 10, 2020
@rpkyle rpkyle linked a pull request May 18, 2020 that will close this issue
@rpkyle
Copy link
Contributor Author

rpkyle commented May 24, 2020

@waralex We're almost done with this issue! I think all that remains is to support assets_external_path.

@rpkyle
Copy link
Contributor Author

rpkyle commented Aug 12, 2020

assets_external_path is now supported, so this issue has been resolved; closing.

@rpkyle rpkyle closed this as completed Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants