-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
generalize lift() and @lift() #3915
base: master
Are you sure you want to change the base?
Conversation
@lift: allow single observable, or no observables at all lift: allow operating on non-observables
function lift(f, args...; kwargs...) | ||
if !any(a -> isa(a, AbstractObservable), args) | ||
# there are no observables | ||
f(args...) | ||
else | ||
# there are observables, but not in the first position | ||
lift((_, as...) -> f(as...), Observable(nothing), args...; kwargs...) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're doing this, then lift
should always return an Observable, no? This way there's a guarantee of what is happening (and lift becomes somewhat type stable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're doing this, then lift should always return an Observable
Why?
It seems natural to return an observable whenever needed – so, whenever there is at least one observable in the arguments.
This nicely fits the conceptual meaning of "lift the passed function to the domain of observables" – surely should be a no-op if no observables are actually passed, right?
bump :) |
bump... |
1 similar comment
bump... |
Not sure if I like enabling non-observable arguments. In what use cases do you get either observable args or not? The attributes in recipes are all converted for example. I find an explicit conversion clearer, but I also understand the convenience aspect. |
I mostly have enduser code in mind, but useful for packages/function implementations as well.
Currently, there are many cases with either completely artificial restrictions, or even error-prone: julia> x = Observable(1)
# I guess I want to add 1 to x...
julia> y = @lift $x + 1
Observable(2)
# ... or maybe I don't
julia> y = @lift $x
ERROR julia> x = Observable([1,2,3])
julia> y = lift(length, x)
Observable(3)
julia> x = [1,2,3]
julia> y = lift(length, x)
3-element Vector{Int64}:
1
1
1
No worries, preferences differ (even though I don't understand the downsides of this generalization). |
IMO the behavior of
Both options will reduce the likelihood of error, which is the biggest problem this PR is attempting to solve. |
@lift: allow single observable, or no observables at all
lift: allow operating on non-observables
Motivation:
this enables preparing plotting arguments without manually handling observable and non-observable cases, like
Fixes #3801.