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

generalize lift() and @lift() #3915

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

aplavin
Copy link
Contributor

@aplavin aplavin commented May 30, 2024

@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

# works no matter if x is a regular value or observable
y = lift(myfunc, x)
# or
y = @lift myfunc($x)

plot(y)

Fixes #3801.

@lift: allow single observable, or no observables at all
lift: allow operating on non-observables
Comment on lines +4 to +11
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
Copy link
Member

@asinghvi17 asinghvi17 May 30, 2024

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)

Copy link
Contributor Author

@aplavin aplavin May 30, 2024

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?

@aplavin
Copy link
Contributor Author

aplavin commented Jun 10, 2024

bump :)

@aplavin
Copy link
Contributor Author

aplavin commented Jun 21, 2024

bump...

1 similar comment
@aplavin
Copy link
Contributor Author

aplavin commented Jul 3, 2024

bump...

@jkrumbiegel
Copy link
Member

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.

@aplavin
Copy link
Contributor Author

aplavin commented Jul 4, 2024

In what use cases do you get either observable args or not? The attributes in recipes are all converted for example.

I mostly have enduser code in mind, but useful for packages/function implementations as well.
This generalization is useful whenever you need to switch between regular values and observables:

  • When writing plotting functions (not strictly "recipes") that accept both. Either convert everything to observables manually, or use this PR implementations. The latter is cleaner, less boilerplate, and works if some downstream paths don't take observables (this path can only be used with regular values ofc, but everything is automatic).
  • When creating a plot and exploring options on the way: should it be static or interactive? Which values should be animated?

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

Not sure if I like enabling non-observable arguments.

No worries, preferences differ (even though I don't understand the downsides of this generalization).
Added these lift and @lift implementations to MakieExtra.jl so that anyone can use them already.

@EdsterG
Copy link
Contributor

EdsterG commented Aug 12, 2024

Not sure if I like enabling non-observable arguments.

IMO the behavior of lift needs to be changed to:

  1. The proposed behavior in this PR
    or
  2. Error on non-observable x

Both options will reduce the likelihood of error, which is the biggest problem this PR is attempting to solve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs discussion/help
Development

Successfully merging this pull request may close these issues.

lift should support non-observables
4 participants