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

eval-free @manipulate macro #11

Closed
stevengj opened this issue Aug 22, 2014 · 4 comments
Closed

eval-free @manipulate macro #11

stevengj opened this issue Aug 22, 2014 · 4 comments

Comments

@stevengj
Copy link
Contributor

Unlike @lift (which can probably be deleted now that we have @manipulate, by the way), I think you can write @manipulate so that it doesn't use eval. Using eval is always a bad idea in a macro, because it makes it extremely brittle, e.g. you can't use the macro inside a function. Following the syntax in #8, just have the macro transform:

@manipulate for x1=params1, x2=params2
    expression
end

into something like

let x1=widget(params1, label="x1"), x2=widget(params2, label="x2")
    display(x1)
    display(x2)
    lift((x1,x2) -> expression, x1, x2)
end

where widget is an overloaded function that creates a default widget (or anything that can be passed to signal) from the parameters of a given type. e.g. widget(::Bool) will create a checkbox, widget(::Range) will create a slider, etcetera.

This has the additional advantage that the available widgets for @manipulate are no longer hard-coded. Any package can provide additional widgets just by overloading widget.

@shashi
Copy link
Member

shashi commented Aug 22, 2014

Cool! This looks like a way better way to do this. Thanks, will work on it tonight. @lift is exported by React, but @manipulate is exported by Interact. So I think there is some value in keeping @lift for code that doesn't involve Interact. Eval in @lift seems like a bad idea now though, but makes it very useful in non-function contexts.

Do you think it's a good idea to introduce a Signalable (better name?) type in React which can be a contract that signal(::Signalable) -> ::Signal will work? Then we can have Widget <: Signalable, and methods that don't risk signal(::Any) failing. I guess this is something you were going for in #1?

@stevengj
Copy link
Contributor Author

I don't think we need a type unless we are going to dispatch on it.

@shashi
Copy link
Member

shashi commented Aug 22, 2014

I forgot to mention we will also have abstract Signal <: Signalable. We will be dispatching on it. This would simplify some things: e.g. there are right now two conditions in widget: isa(w, Widget) and isa(w, Signal) which get handled the same way. Instead we could have had a signle isa(w::Signalable).

I'm skeptical about the name Signalable though.

@shashi
Copy link
Member

shashi commented Aug 22, 2014

I guess this issue is now addressed by 4f920bf

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

No branches or pull requests

2 participants