param icon indicating copy to clipboard operation
param copied to clipboard

Add bind function

Open philippjfr opened this issue 3 years ago • 4 comments

Adds param.bind function which acts like functools.partial but evaluates the current value of a parameter.

  • [ ] Add tests
  • [ ] Add docs
  • [ ] Decide whether to offer hooks for param.depends and param.bind so that they can accept other objects and interpret them as parameters (making them behave like pn.depends and pn.bind).

philippjfr avatar Feb 23 '21 10:02 philippjfr

@jlstevens suggested the hooks could take the form of:

{class_to_check_isinstance:['param1', 'param2',...], ...}

However that doesn’t seem sufficient, e.g. in panel I actually create synthetic parameters around ipywidgets:

    from .pane.ipywidget import IPyWidget
    if IPyWidget.applies(arg) and hasattr(arg, 'value'):
        name = type(arg).__name__
        if name in ipywidget_classes:
            ipy_param = ipywidget_classes[name]
        else:
            ipy_param = param.parameterized_class(name, {'value': param.Parameter()})
        ipywidget_classes[name] = ipy_param
        ipy_inst = ipy_param(value=arg.value)
        arg.observe(lambda event: ipy_inst.param.set_param(value=event['new']), 'value')
        return ipy_inst.param.value

philippjfr avatar Feb 23 '21 11:02 philippjfr

I think a dictionary is always appropriate as you have a match condition then a way to get the parameters. So maybe a bunch of things could be supported:

{class1_to_check_isinstance:['param1', 'param2',...], 
class2_to_check_isinstance:function_returning_parameter_list_given_instance, 
predicate_function:function_returning_parameter_list_given_instance
... }

Then you can use as complicated a format as necessary.

jlstevens avatar Feb 23 '21 11:02 jlstevens

Looks good to me but probably needs watch support as suggested in https://github.com/holoviz/panel/issues/1999.

jbednar avatar Feb 23 '21 16:02 jbednar

I think a watch argument is definitely required now panel supports it: I don't think having the panel and param versions of bind diverge would be a good thing.

jlstevens avatar Mar 01 '21 13:03 jlstevens

@philippjfr do you need this in 2.0?

maximlt avatar Apr 05 '23 09:04 maximlt

@philippjfr , up to you to declare this ready for review.

jbednar avatar May 12 '23 21:05 jbednar

I've decided simply to support a list of hooks instead of the dictionary format as I don't much like the idea of mixing types and predicate functions in the dictionary suggestion, because it makes the ordering ambiguous, while the list of hooks simply iterates in order until it is either exhausted or returns a Parameter or function with dependencies.

philippjfr avatar Jun 22 '23 09:06 philippjfr

I really like pn.bind in Panel, specially that it's similar to functools.partial. I think that makes it really easy for users to leverage functions from other libraries (imagine str.upper being replaced by some model returning an object Panel supports):

image
def bind(function, *args, watch=False, **kwargs):

I haven't been super convinced by using pn.bind with watch=True though:

  • it adds a parameter to the signature of bind which weakens the functools.partial equivalence:
  • it's used for registering callbacks with side-effects only but when I see a stray line like pn.bind(func, ..., watch=True) in an example, possibly quite far from the definition of func and the objects it's being bound to, I find that hard to spot and the intention pretty unclear. Panel users could already register them decorating a function with @pn.depends(..., watch=True) which at least had the advantage of making it clear that this was a side-effect callback (ignoring the general confusions users have with @pn/param.depends(...) and watch=True). Panel has been more and more recommending pn.bind instead of @pn.depends, in the case of side-effects callbacks I question whether this is an improvement. Even if the introduction of the reactive API and soon pn.interactive makes it less likely to have to write such side-effects callbacks, it won't cover all the cases though and so I believe there's a need for a better API.

I think what I'd prefer to have would roughly be something like:

watchers = param.watch?(func, panel_widget, param_obj, bound_func, ...)
# or
watchers = param.bind(func, panel_widget, param_obj, bound_func, ...).watch?()

Returning an object that can be used to unregister the watchers.

maximlt avatar Jun 23 '23 11:06 maximlt

I fully agree, and strongly regret adding watch to bind. The main problem we face though, is that deprecating it is quite difficult and adding a diverging version of bind to param is confusing.

philippjfr avatar Jun 23 '23 11:06 philippjfr

Please also consider supporting param.bind/param.depends to a Parameterized class will bind to its .value parameter if it exists. And maybe even to its .object or .objects parameter if it exists.

This will solve many issues

  • I experience users being very confused by seeing a mix of binding directly to a widget and then to the .param.value of a Parameterized class.
  • users cannot easily make widgets in Panel today because widgets are not simply Parameterized classes with a .value Parameter. They have to inherit from Widget.
  • why cant you use a Pane (or even Layout) as input to pn.bind/ pn.depends in the same way as a widget?

MarcSkovMadsen avatar Jun 23 '23 14:06 MarcSkovMadsen

This new implementation allows providing hooks that convert arguments to dependencies so you could do something like this:

def transform_pane_obj_dep(obj):
    return obj.param.object if isinstance(obj, PaneBase) else obj

param.parameterized.register_depends_transform(transform_pane_obj_dep)

It definitely isn't param's job to register something like this though, so please file an issue on Panel instead.

philippjfr avatar Jun 23 '23 14:06 philippjfr

Hooks are an implementation detail.

I really Think that there should be only one .bind function. Having param.bind and pn.bind with different behaviour is just so confusing to users in practice.

And if .bind could be used as an annotation and .depends could be ditched or vice versa it would again make Panel and HoloViz easier to explain.

MarcSkovMadsen avatar Jun 23 '23 20:06 MarcSkovMadsen

That's the whole point of this PR, to have one version of depends and bind with the same behavior. I'm just saying that while param defines the ability to define hooks, the actual implementation will have to be in Panel so I'm asking you to open an issue there.

philippjfr avatar Jun 23 '23 20:06 philippjfr

That is not what I'm asking 😉. Im asking to just have param.bind to replace param.depends as well as pn.bind and pn.depends

MarcSkovMadsen avatar Jun 24 '23 03:06 MarcSkovMadsen

Got you, I was referring to this part of your post:

why cant you use a Pane (or even Layout) as input to pn.bind/ pn.depends in the same way as a widget?

I will see what it would look like to allow binding string parameters to methods like depends.

philippjfr avatar Jun 24 '23 05:06 philippjfr

I've never understood why it was not technically possible to just have

  • param.watch
  • param.bind or param.depends (and without watch). Why is it not technically possible to implement a function that can work both as a function and as a decorator. That would simplify so much.

MarcSkovMadsen avatar Jul 29 '23 05:07 MarcSkovMadsen

@MarcSkovMadsen, @maximlt and @jbednar I've now added a description of the full public API surface of reactive. Please consider the naming and the potential for those names to clash with the API of the objects that are being wrapped.

philippjfr avatar Jul 29 '23 15:07 philippjfr

Why is it not technically possible to implement a function that can work both as a function and as a decorator.

Maybe try implementing that, and if it behaves as you wish, show us! I haven't found a way to do that without requiring an awkward extra set of parentheses at some point.

jbednar avatar Jul 29 '23 17:07 jbednar

That part is doable but the last time @jbednar discussed this the problem was that the semantics of depends and bind are still quite different.

philippjfr avatar Jul 29 '23 18:07 philippjfr

reactive API

I love the reactive API, but I do worry that the set of methods involved is fairly large, likely to conflict, and hard to remember.

Could we maybe put them all into a namespace like .rv so there is only one point of conflict, so that people can detect when we're using one of our methods and not one from the underlying object, and so that the namespace can be documented? At that point we can add all the methods we want.

bool_: Casts the object to bool (equivalent to .pipe(bool)), needed because bool must return a boolean value. is_: Allows performing identity comparison on the underlying object, e.g. obj is None

I think is is a keyword, hence the _, but can't we have methods named bool?

set_display: Sets display options on the reactive object (corresponds to kwargs passed to .interactive(...) in the original hvPlot interactive API)

display_opts?

set: If the input to the expression was a scalar value (i.e. not a reference such as Parameter or bound function) then this allows updating the input to the expression

I'd need an example to understand set; is it a way to "fake" having a parameter as input? How does the user specify that initial value? param.reactive.wrap(3.2)? Whatever that wrapper is called then maybe this name could evoke that name. set has so many meanings that it is hard to remember what this one would do.

the semantics of depends and bind are still quite different.

I think I'm a little lost here. Yes, depends and bind have very different semantics; bind removes the argument from the function list, while depends declares a dependency but does not affect the argument list. Sometimes you want one, sometimes the other. Is the request to choose one of those semantic alternatives and hide the other? That would be difficult, because binding makes sense for bare functions where the input is an argument, while expressing a dependency makes sense for a class method where the input is obtained from object attributes. Or is the idea to make the method require an explicit argument that is then bound, not using any class attributes? That seems confusing. I may be missing something about the proposed solution.

jbednar avatar Jul 30 '23 16:07 jbednar

Could we maybe put them all into a namespace like .rv so there is only one point of conflict, so that people can detect when we're using one of our methods and not one from the underlying object, and so that the namespace can be documented? At that point we can add all the methods we want.

Not a huge fan, especially with that naming. If we do decide on a namespace I would take that opportunity to avoid having the same namespace for methods that operate on the proxied object and methods that operate on the reactive object itself.

I think is is a keyword, hence the _, but can't we have methods named bool?

This is true, should rename it.

display_opts?

As a method?

I'd need an example to understand set; is it a way to "fake" having a parameter as input? How does the user specify that initial value? param.reactive.wrap(3.2)? Whatever that wrapper is called then maybe this name could evoke that name. set has so many meanings that it is hard to remember what this one would do.

This is correct but the API is simply:

> reactive_float = param.reactive(3.2)
> multiplied_float = reactive_float * 2
> multiplied_float.set(1.2)
> multiplied_float
2.4

I could see set_input as an alternative.

Is the request to choose one of those semantic alternatives and hide the other?

Yes, that is Marc's suggestion or rather somehow overload the semantics so they can be used interchangeably.

philippjfr avatar Jul 31 '23 09:07 philippjfr

In a meeting just now we agreed that the "terminating methods" that the original hvplot.interactive implementation used do not really make sense in a Param context and we really want to explicitly force a user to display the object using the Panel API. Below are the equivalent implementations for the old terminating methods:

.layout

pn.panel(reactive_expr)
# or equivalently
pn.pane.ReactiveExpr(reactive_expr)

.panel

pn.panel(reactive_expr).object
# or equivalently
pn.pane.ReactiveExpr(reactive_expr).object

.widgets

pn.panel(reactive_expr).widgets
# or equivalently
pn.pane.ReactiveExpr(reactive_expr).widgets

.output

pn.panel(reactive_expr).object
# or if you want HoloViews to render the object
hv.DynamicMap(reactive_expr)

philippjfr avatar Jul 31 '23 16:07 philippjfr

Based on our discussion the other day I've moved all custom methods that were originally on reactive itself on a new namespace called rx which is short for "Reactive eXpression". This namespace has the following methods:

Operations

  • .rx.bool(): Tests the truthiness of the object
  • .rx.is_(): Tests the object identity against another object.
  • .rx.is_not(): Tests the object identity against another object.
  • .rx.is_too(): Juvenile response to previous method, illustrating the power of the edit button on GitHub
  • .rx.len(): The length of the expression
  • .rx.pipe(): Allows applying an arbitrary function with static or reactive arguments to the object.
  • .rx.when(): Generates a new expression that only updates when the provided dependency updates.
  • .rx.where(): Returns either the first or the second argument depending on the current value of the expression.

Input/Output

  • .rx.observe(): Adds an observer which is passed the current value whenever it updates
  • .rx.resolve() : Formerly .eval() this resolves/evaluates the current value of the expression
  • .rx.set(): Sets the literal value input of an expression. While people raised some concerns about this naming I still think is clearest and aligns with concept of a state setter in React.

philippjfr avatar Aug 03 '23 09:08 philippjfr

@jbednar @maximlt and anyone else who is interested, please review.

philippjfr avatar Aug 29 '23 13:08 philippjfr

The naming of this seems backward, from the description. If I have an expression X that "observes" some Y, I expect X to respond to Y, whereas here it seems like Y is responding to X. Maybe .rx.observer()?

Don't really get this, .watch has exactly the same semantics and isn't called .watcher.

Hmm. I still have the same concerns with the name "set", coming back to it a month later.

Not sure I like .update much, I'd be okay with something like .set_input I suppose.

I.e., partial should be able to be applied repeatedly, one argument at a time ("currying"), each time returning a "more bound" function until all arguments are fully bound.

Fixed

I think we discussed and must have rejected this possibility during the design sessions, but coming to it fresh I would again suggest supporting rx.call either instead of or as an alias for resolve. It's just a natural counterpart to how we work with reactive bound functions. () for both!

I put a poll up to decide between these and people didn't vote for __call__, I agree with them.

It looks like or behaves appropriately I.e., the result of or is a reactive expression with the appropriate value.

This isn't true, reactive is always truthy, your example simply happened to assume truthiness.

think people are going to be confused about the difference between .bind and .reactive, since both create "reactive" objects. Can't we add .rx to .bind and support call on both so that people don't have to care about the difference? I think I've asked this before and you explained the reasons but I must have forgotten them.

I'm quite wary of adding API that make it appear like two objects are the same when they really aren't. Either param.bind should just go ahead and return a reactive expression OR we should continue making the distinction, superficially papering over the distinction is just going to confuse more people.

philippjfr avatar Sep 17 '23 09:09 philippjfr

The naming of this seems backward, from the description. If I have an expression X that "observes" some Y, I expect X to respond to Y, whereas here it seems like Y is responding to X. Maybe .rx.observer()?

Don't really get this, .watch has exactly the same semantics and isn't called .watcher.

I don't use .watch, but I guess I have the same complaint about that too, then! Sounds like the semantics of "watch" is thus "register_watcher", i.e., the opposite of "watch". A verb method on an object normally means an action invoked on that object, not an action another object is doing on this one, but sounds like it's too late to change in any case.

reactive is always truthy

So that means none of the logical operators work? Does that need to be documented specially?

think people are going to be confused about the difference between .bind and .reactive, since both create "reactive" objects. Can't we add .rx to .bind and support call on both so that people don't have to care about the difference? I think I've asked this before and you explained the reasons but I must have forgotten them.

I'm quite wary of adding API that make it appear like two objects are the same when they really aren't. Either param.bind should just go ahead and return a reactive expression OR we should continue making the distinction, superficially papering over the distinction is just going to confuse more people.

I think they share enough similarities that this is a problem already, which is what I'm concerned about. Is making them be the same problematic?

jbednar avatar Sep 18 '23 03:09 jbednar

A verb method on an object normally means an action invoked on that object

Don't see how watch and observe don't meet that requirement, the action is to watch or observe changes on the object, the thing you are registering is the watcher. This naming is quite common across libraries that allow registering callbacks. I also really can't see how register_watcher is the opposite of watch, registering the watcher just seems like a technical detail required to watch for changes.

So that means none of the logical operators work? Does that need to be documented specially?

Yes, that seems like a good idea. Reactive doesn't support operator keywords (i.e. and, or, not, in and is), control flow keywords (i.e. if, elif, else), ternary conditional expressions (i.e. a if condition else b), or iteration keywords (i.e. for, while, break, continue, else). That last one isn't quite true because it does implement an iterator interface, but that has the limitation of not allowing variable length unpacking.

I think they share enough similarities that this is a problem already, which is what I'm concerned about. Is making them be the same problematic?

Yes, unfortunately, we can't really resolve a reactive expression on __call__ because that would override the __call__ of the underlying object (if it exists).

philippjfr avatar Sep 18 '23 10:09 philippjfr

I guess your point about watch is really about who is the subject of the verb, for watch you might expect that the object is doing the watching. I think the fact that this is a common convention is enough to justify it, but if you want some silly sophistry to justify it then read the signature of a method which is watch(self, ...) which reads fine :)

philippjfr avatar Sep 18 '23 10:09 philippjfr

I'd like to merge this so we can start testing it as part of a beta release. There's a few outstanding items that I promise to address:

  • Add a clear explanation of the "Why"
  • Document the keywords and expressions that don't work directly with reactive
  • Cross-link Reactive docs

philippjfr avatar Sep 21 '23 16:09 philippjfr

Final changes we decided on:

  • Renaming .rx.set -> .rx.set_input
  • Renaming .rx.observe -> .rx.watch

We have also decided that to make the parallels between reactive, bind and Parameter explicit by adding the .rx namespace to all of them. Additionally .rx() can be used to turn bind and Parameter types into a reactive expression (for reactive objects this is a no-op).

philippjfr avatar Sep 22 '23 15:09 philippjfr