JuliaGizmos / InteractBase.jl

Build interactive HTML5 widgets in Julia
Other
27 stars 23 forks source link

@eval code generation is kind of confusing! #112

Open rafaqz opened 6 years ago

rafaqz commented 6 years ago

I was trying to track some broken styling in inputs.jl, but the eval blocks make it a little hard to track what is happening! I'm wondering if it is worth having them at all, in terms of the obfuscation vs DRYness ratio they deliver.

You could get a lot off the way by just extracting a few shared functions, say in slider.jl:

slider(WT::WidgetTheme, vals::AbstractArray, formatted_vals = format.(vec(vals)); value = medianelement(vals), kwargs...) =
    build_slider(slider, WT, vals, formatted_vals, value)

rangeslider(WT::WidgetTheme, vals::AbstractArray, formatted_vals = format.(vec(vals)); value = medianelement(vals), kwargs...) =
    build_slider(rangeslider, WT, vals, formatted_vals, value)

function build_slider(f, WT, vals, formatted_vals, value)
    T = Observables._val(value) isa Vector ? Vector{eltype(vals)} : eltype(vals)
    value isa AbstractObservable || (value = Observable{T}(value))
    vals = vec(vals)
    indices = axes(vals)[1]
    f = x -> _map(t -> searchsortedfirst(vals, t), x)
    g = x -> vals[Int.(x)]
    ObservablePair(value, f = f, g = g).second

    value = build_value(vals, value)
    index = build_index(vals)

    wdg = Widget(f(WT, indices, formatted_vals; value = index, kwargs...), output = value)
    wdg["value"] = value
    wdg
end

Anyway, that is just my 2 cents! And mostly because I imagine I'll be reading this code a lot more, because its so useful :)

piever commented 6 years ago

The slider stuff was actually very tricky to get right because it needs to work with arbitrary AbstractVector of potentially "unitful" quantity but at the same time I wanted to keep simple things (slider of consecutive integers) simple (meaning no extra julia - javascript communication) and accomodate both interfaces (native html and the nouislider API).

I don't mind so much the@eval code generation, but it's probably important to comment the code to clarify what methods are called and how: for slider this is actually quite a tricky pipeline, as it's first getting the indices of the array and the values formatted by Julia, then calling the method for indices and formatted values, which in turns calls the method for AbstractArray{<:Integer} and adds some javascript to show the formatted values correctly.

rafaqz commented 6 years ago

I was trying to write it as a pure demonstration refactor with no changes to functionality, but I didn't test it so maybe it is actually different.

I'm also fine with the @eval but it does take a lot longer to understand.