Ensure all watchers are executed in scheduling order #323
Conversation
|
One way to make this PR cleaner would be to add a context manager for setting and unsetting the BATCH_WATCH flag. |
philippjfr
force-pushed
the
batch_watch_ordered
branch
from
a4e9c7d to
dca2a62
Compare
|
Ready to review/merge. |
| @@ -1215,6 +1238,7 @@ def set_param(self_, *args,**kwargs): | |||
| positional arguments, but the keyword interface is preferred | |||
| because it is more compact and can set multiple values. | |||
| """ | |||
| BATCH_WATCH = self_.self_or_cls.param._BATCH_WATCH | |||
There was a problem hiding this comment.
Just following the style of the original variable name, not sure it has to be capitalized but if we decide to change I would change it everywhere.
|
Quick question on this: does the watcher get the event itself in its arguments? I would say, that an event which is going to cause another like event of a different value to be spawned should explicitly set the event to be ignored downstream. That way, if you want a "less than 4" filter on an element as you described, other watchers never see the value 5, even temporarily. This leaves it up to the programmer to think about whether it makes sense for an event to propagate or not. |
Yes it does, and I agree with you. I think a watcher should be able to invalidate an event it receives so that downstream watchers never see it. That said maybe a better solution would be to offer a way to define preprocessors on parameters so the value is validated (or in this case clipped) before it is set and triggers downstream events. I can't quite envision if there are other scenarios where you might want a watcher to invalidate an event it receives. |
This way means having a different type of watcher, which may be confusing. I suppose it could be a very simple function valid(val) -> True/False. I can imagine scenarios, such as the new value caused a procedure to fire which failed, so it is OK to set that value (it's not invalid, only failing at the current moment and set of other values) but follow-on procedures should not run. You could argue this case is moot, any follow-ons should respond the a change in a separate output area... |
|
This PR also happens to fix holoviz/panel#143. I think we should merge this PR and then file an issue to discuss any suggestions related to letting users invalidate events. |
|
That's certainly fine with me. |
|
I think I'll go ahead and merge this PR so we can test it. I agree with Jim that the earlier discussion suggests that this is the right approach. |
Fixes #322