Add functionality to stream from parameterized objects and methods #2961
Conversation
|
Tempted to leave testing and docs until there is a param release to update to but I'll defer to your judgement @jlstevens. If you agree this is ready to review/merge. |
|
I'm not sure whether I want to merge it yet but one thing that would make me happier is if there were a param version check so that the new execution path would only apply for current param dev versions or higher. |
holoviews/core/spaces.py
Outdated
| @@ -749,14 +755,34 @@ def __init__(self, callback, initial_items=None, **params): | |||
| 'and no longer needs to be specified.') | |||
| del params['sampled'] | |||
|
|
|||
| super(DynamicMap, self).__init__(initial_items, callback=callback, **params) | |||
| invalid = [s for s in self.streams if not isinstance(s, Stream)] | |||
| # Validate streams by ensuring parameterized objects and methods are wrapped in ParamStream | |||
There was a problem hiding this comment.
How about a utility here? Something like:
valid, invalid = util.parameterized_streams(streams)
I'm not quite sure what you need invalid for though...
There was a problem hiding this comment.
If you look below you'll see that invalid streams will raise errors. Happy to factor that out if you want.
There was a problem hiding this comment.
Oops missed that. I think the utility could raise the exception for invalid inputs.
| @@ -1263,6 +1263,13 @@ def get_param_values(data): | |||
| return params | |||
|
|
|||
|
|
|||
| def is_param_method(obj): | |||
There was a problem hiding this comment.
Seems like something that could live in param...
There was a problem hiding this comment.
Agreed, I'll add that in a PR, I'm also duplicating it in panel, so that would definitely make sense.
holoviews/streams.py
Outdated
| parameters=parameters, **params | ||
| ) | ||
| for p in self.parameters: | ||
| self.parameterized.param.watch(self._listener, p) |
There was a problem hiding this comment.
I don't see any use of unwatch yet. Surely there are situations where clean up is needed?
There was a problem hiding this comment.
Instead of defining _listener, wouldn't a lambda be sufficient?
holoviews/streams.py
Outdated
|
|
||
| @property | ||
| def contents(self): | ||
| if self.is_method: return {} |
There was a problem hiding this comment.
If it isn't a method, I assume the contents is based on what ParamValues used to do? I'm not sure yet, but I expected this stream to be just for triggering, i.e always return {}. That might suggest to me that we can leave ParamValues as it was an make ParamStream its own thing...
There was a problem hiding this comment.
In other words maybe (decorated?) parameterized methods should have their own stream class independent of arbitrary parameterized things...
holoviews/streams.py
Outdated
|
|
||
| # Backward compatibility | ||
| def ParamValues(*args, **kwargs): | ||
| param.main.warning('ParamValues stream is deprecated, use ParamStream instead.') |
philippjfr
force-pushed
the
param_watch
branch
from
df7dde8
to
b4c6aa2
Compare
|
Thanks for splitting it into I am now happy to merge even though there are two items left on the TODO list (unit tests and docs) as we can now test this out with a dev release. Of course, we will then have to address those two items (and any new issues we encounter) later. |
Requires holoviz/param#253 to work
param.depends