[WIP] Defined pre_set_hooks and post_set_hooks for Parameters

[jlstevens]

This PR is a prototype that replaces set_hook on Number with a more general system of pre- and post- setting hooks. This may be useful for things like linking streams or eventual unit support in param.

An example of what this might be useful for is shown in holoviz/holoviews#1740.

I'm not quite sure about the appropriate signatures for the hooks right now and this PR will probably need quite a bit of discussion before we are agreed on the appropriate approach.

[jlstevens]

I should also note that this PR relates to issue #170.

[coveralls]

Coverage decreased (-0.06%) to 74.587% when pulling 2cab62e on set_hooks into a52edf5 on master.

[ceball]

I should also note that this PR relates to issue #170.

Also: #160

[jbednar]

Seems like a very reasonable extension, but note that it adds two slots to every parameter, and it seems unlikely that this mechanism will be used often, so that seems like a bit of a waste. It would be awkward to do it in a single parameter (e.g. a tuple of the pre,post lists) but wouldn't be that bad; is it worth it?

[jlstevens]

Tuples are fine with me. That said, what makes slots expensive to use? Memory consumption?

[jbednar]

Isn't each one given a fixed amount of reserved memory space, per parameter?

[ceball]

I quickly scanned the diff in hv. I may well be misreading that diff (and I don't know the classes there generally), but it appears to be effectively adding an 'instance-level' update method into the 'class-level' hook list (https://github.com/ioam/holoviews/pull/1740/files#diff-deb386f2061a9a3f41d4982d6a4f7926R203).

So, every time you make a new instance, won't a new instance-level update method be added to the class-level hook list, and then when the parameter is set (on the class or on any instance), all those update methods will be called?

[ceball]

Oh, actually I see it's making a new list each time - but in that case, when the parameter is set on the class or on any instance, it'll be the most recently created instance's update method that'll be called. (Assuming I read it right.)

[ceball]

Seems like a very reasonable extension,

Many things seem like they would be reasonable extensions to param. Poor param! ;)

I've often vaguely wanted something like this, but never actually needed it in the end. (Or maybe I gave up on doing something cool?) I.e. I want to add this feature, and I don't want to add it!

[ceball]

[Note] that it adds two slots to every parameter, and it seems unlikely that this mechanism will be used often, so that seems like a bit of a waste. It would be awkward to do it in a single parameter (e.g. a tuple of the pre,post lists) but wouldn't be that bad; is it worth it?

Isn't each [slot] given a fixed amount of reserved memory space, per parameter?

Ah, slots. It's been a long time since I was working on any of this, so don't necessarily believe anything I say, but...

I'd guess it would be 64 bits per slot per parameter object for most people (one python reference - i.e. one pointer - per slot). And maybe the same again for the list it points to? However, most uses of param don't result in many parameter objects - right? Parameter objects are created by Parameterized classes. Usually the number of instances of classes is much bigger than the number of parameters. (And really we don't expect there to be all that many Parameterized instances, either.)

I think the benefit of slots in Parameter comes from e.g. not allowing mistakes in Parameter declarations (defualt=4)...because Parameter doesn't have Parameters ;) Oh, and also it usually prevents anyone adding more stuff to Parameters because they can't summon the willpower to deal with slots ;)

Slots do save memory (vs object with dict), and they do make attribute access faster (vs regular attribute lookup, or because they allow optimized access based on memory address - which could additionally avoid the GIL) but I doubt any of that helps us much. Slots definitely don't save my memory or time whenever I work on param...

[jlstevens]

So, every time you make a new instance, won't a new instance-level update method be added to the class-level hook list, and then when the parameter is set (on the class or on any instance), all those update methods will be called?

That is right for what I want in HoloViews, though really I only want this hook called when instance parameters are set. This is actually a bit of an annoyance - once you set the hook in the constructor once, it exists at the class level so if you make a new instance, the hook then exists and is called in the constructor of the second object you created, stopping you from setting parameters in the constructor.

I think I could easily update this PR to only apply hooks at the instance level, just by checking whether obj is a class or not before applying the hooks. Does this also seem more useful to you?

[jbednar]

Adding slots for this sounds fine in terms of memory; I was thinking they would take space per instance, but if it's per class, I don't think it's worth worrying about. So never mind on the tuple idea; that's just making things harder for ourselves. I'm happy to add this feature as long as we either have found a very clear use case for it by the time of the next release, or we mark it experimental until we have done so.

[philippjfr]

I'm +1 on this. It can also eventually be used to replace View parameters I added to paramnb and paramBokeh or at least unify them in some way.

[jbednar]

It can also eventually be used to replace View parameters I added to paramnb and paramBokeh or at least unify them in some way.

It would be great to eliminate those, so that objects can be specified using Param only. View parameters work well, but they violate separation of content and presentation, so I'm very much in favor of this PR if indeed it does allow the specification of View parameters to be done at the Param level.

[ceball]

Do the votes in favor mean positive reviews of how the implementation works now and you want to merge, or of the idea of pre- and post-set hooks in general?

Can someone point me to an example using View parameters?

[philippjfr]

Do the votes in favor mean positive reviews of how the implementation works now and you want to merge, or of the idea of pre- and post-set hooks in general?

My +1 was mostly about the idea, I'll look at the implementation now.

Can someone point me to an example using View parameters?

There are some examples here: https://github.com/ioam/paramnb/blob/master/doc/AdditionalFeatures.ipynb

In essence View parameters declare a post-set hook, which are used to update the display output whenever you set the parameter.

[philippjfr]

Should update this docstring since set_hook has now been removed.

[philippjfr]

Okay I see the implementation is very simple. The main question I have is about instance level callbacks. I know you encountered the same issue when you prototyped this so I want to establish what the recommended approach will be. The problem is that you often want to define a callback per instance, e.g. when you create two parameterized instances to use as widgets or two stream instances you don't want setting the parameter on one instance to trigger the set hook installed by another instance. In paramnb/parambokeh I used the object id to trigger only the set hooks for a particular instance, how would you recommend this should be handled now?

[ceball]

Thanks for the link about View parameters - will check those out.

Okay I see the implementation is very simple.

Unfortunately I still don't understand it :(

The main question I have is about instance level callbacks

Yes, I'm confused about what the desire/intent is here.

@jlstevens asked:

I think I could easily update this PR to only apply hooks at the instance level, just by checking whether obj is a class or not before applying the hooks. Does this also seem more useful to you?

I don't think so, no. But I'm not sure about the intent. Can I just double check again that I understand what you are trying to achieve? You want to have something happen to all instances of a class when a parameter is set on any one of them?

And can I double check what your implementation does/what you want it do?

1. Right now in your implementation, the hook list you've created is at the parameterized class level. You are storing one single list of things to call when a parameter is set, whether that parameter is set via the class or it's set on any instance. If the parameter is set on the class, each thing in the parameter's hook list will be called with the class and the new value. If the parameter is set on instance x, each thing in the parameter's hook list will be called with x and the new value. If the parameter is set on instance y, each thing will be called with y and the new value.

2. Your proposed modification is to still have the hook list stored in the parameter (i.e. at the parameterized class level), but when the parameter is set on the class you will skip calling the hook list functions - you will only call them when the parameter is set on an instance (any instance). So again when the parameter is set on instance x, each function in the parameter's hook list will be called with x and the new value. If the parameter is set on instance y, each thing will be called with y and the new value.

If all of the above is right (is it?), I think I'd instead prefer the same kind of semantics as for setting and getting parameter values in general. That is, I'd want to be able to set pre-/post-set hooks per instance or per class, and to know which I'm doing. I would want to be able to say something like these things for class C and instances of it:

i. Whenever parameter a is set on instance x, I want function f to be called (as f(x,new_val) plus possibly including other things like the parameter name but let's discuss that later). Setting a on instance y would do nothing. Setting a on class C would do nothing.

ii. Whenever parameter a is set (whether on the class C or on an instance), I want function f to be called (with the class/particular instance and new value as arguments). Setting a on instance x would result in f(x,new_val). Setting a on instance y would result in f(y,new_val). Setting a on class C would result in f(C,new_val).

I don't fully understand your hv example (sorry I still have not tried it out yet!), but if you wanted something to happen to all instances whenever the value of a parameter is set on one instance, you could achieve that by adding one function per instance (e.g. an instance method from each instance) to the class-level hook list.

In paramnb/parambokeh I used the object id to trigger only the set hooks for a particular instance,

@philippjfr could you point me to that? Sorry to be lazy and keep asking for pointers but it's tricky for me to keep track of stuff right now.

how would you recommend this should be handled now?

I would expect the class-level hook list to be overridden and stored at the instance level instead.

Unfortunately I still haven't got round to gathering/trying out/understanding the various supplied examples yet to see what it is that actually needs to be supported; I don't want to hold up this PR and be an annoying bottleneck, but I also want to try to protect param a bit.

[philippjfr]

Unfortunately I still haven't got round to gathering/trying out/understanding the various supplied examples yet to see what it is that actually needs to be supported; I don't want to hold up this PR and be an annoying bottleneck, but I also want to try to protect param a bit.

No I think this an important discussion since I expect this functionality to be widely used by both holoviews streams and paramnb/parambokeh and we really want to get it right at the param level. Thanks for outlining the various issues so neatly. I agree with everything you've said and setting the hooks at the class/instance level makes sense. Here is what I did in paramnb: view.py#L22

In that version I use a dictionary of callbacks indexed by the instance/class object id, which I store on the parameter itself. I then manually register a callback for each instance. Storing and setting it on the parameterized class/instance as you suggest is probably preferable though if we add this functionality to param itself.

[jbednar]

What Chris outlined makes very good sense to me. Chris, would you be able to implement that, or would you want Philipp (as someone who has already done the same thing one level up, and has a specific use case for it) or Jean-Luc (as the author of this PR, and also has a specific use case) to do it?

[ceball]

Yes, I'd be happy to continue the implementation once I've understood Jean-Luc's and Philipp's examples (they are likely to have thought of doing things that wouldn't occur to me, and so far I've only looked at the examples superficially). So I'll study the examples mentioned in the comments in more detail (along with another widget example that came up recently in another project) and then implement the above if it seems to fit.

[jlstevens]

You want to have something happen to all instances of a class when a parameter is set on any one of them?

That was my original goal but as Philipp points out, hooks specific to each instance would also be useful.

Right now in your implementation, the hook list you've created is at the parameterized class level. You are storing one single list of things to call when a parameter is set, whether that parameter is set via the class or it's set on any instance. If the parameter is set on the class, each thing in the parameter's hook list will be called with the class and the new value. If the parameter is set on instance x, each thing in the parameter's hook list will be called with x and the new value. If the parameter is set on instance y, each thing will be called with y and the new value.

That sounds about right. As mentioned earlier, the problem with setting hooks at the class level is you trigger the hooks when trying to set parameters via constructors. There must be a better way than the simple thing I attempted to do here.

Your proposed modification is to still have the hook list stored in the parameter (i.e. at the parameterized class level), but when the parameter is set on the class you will skip calling the hook list functions - you will only call them when the parameter is set on an instance (any instance). So again when the parameter is set on instance x, each function in the parameter's hook list will be called with x and the new value. If the parameter is set on instance y, each thing will be called with y and the new value.

Correct.

Your parts i and ii outline the per-instance approach (which I didn't attempt).

I don't fully understand your hv example (sorry I still have not tried it out yet!), but if you wanted something to happen to all instances whenever the value of a parameter is set on one instance, you could achieve that by adding one function per instance (e.g. an instance method from each instance) to the class-level hook list.

That is fine, all instance can point to the same function but with the updated suggestion each instance could point to a different function.

So in summary, I am in favor of per-instance hooks which isn't currently in this PR. My goal was to do something simple to start a discussion and I think it has now done that job! :-)