JuliaGizmos / InteractBase.jl

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

slider fails with Integer range #142

Closed hhaensel closed 5 years ago

hhaensel commented 5 years ago

Yesterday I stumbled across a stange misbehaviour of sliders with Integer ranges.

slider(5:10) will not evaluate correctly, whereas slider(collect(5:10)) does. The reason is that the slider method from Widgets, which adds the backend, will directly jump to the slider method, which generates the ui. (Thanks to @tkoolen and @stevengj who helped me understand the issue!) To prevent this, I propose to add another method

slider(vals::AbstractRange{<:Integer}, args... ; kwargs...) = slider(collect(vals), args... ; kwargs...)

One might add a hint somewhere that calling the backend directly would need a collect for Integer ranges.

piever commented 5 years ago

Well spotted, that method is incorrect. Still not 100% sure why it gets called directly (as you are not passing formatted_vals) but in any case I should restrict the signature and handle the case where vals doesn't start at one.

piever commented 5 years ago

Could you check out #143 and see if it fixes things for you?

hhaensel commented 5 years ago

I checked #143, but it doesn't fix the problem, as typeof(5:10) is a subtype of AbstractUnitRange. The reason, why the wrong method is called, is well explained here. I brought up the problem in discourse because I wanted to understand the misbehaviour of the slider. Steven explains, why leaving out the optional argument doesn't help to chose the proper method. In any case, it's desireable to be able to call slider with manually formatted values. So, I would propose to either go with my solution (which doesn't really hurt) or to rename the last slider method, e.g. _slider. There might, of course, be other solutions - I still consider myself a Julian beginner.

piever commented 5 years ago

Thanks for the link, I now understand the dispatch issue. What I meant is: does #143 fix the slider in practice, i.e., does it work for you? The method still gets called but should now behave correctly for any AbstractUnitRange input.

hhaensel commented 5 years ago

Sorry for my first answer. I had only tested at the REPL, because my WebIO installation was broken, and so I couldn't check that it was definitely working. P.S.: The update of any observable connected to a slider only works after restarting the notebook kernel once. I see this with two different Windows installations of Julia.