From 80176f129cf90dc2e8a8351a03b7ed0fec17c0b7 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 15:34:59 +0100 Subject: [PATCH 01/24] Added core.util.argspec utility --- holoviews/core/util.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 5b63901f98..8401d18b20 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -1,10 +1,12 @@ import os, sys, warnings, operator import numbers +import inspect import itertools import string, fnmatch import unicodedata import datetime as dt from collections import defaultdict, Counter +from functools import partial import numpy as np import param @@ -99,6 +101,33 @@ def deephash(obj): generator_types = (izip, xrange, types.GeneratorType) + +def argspec(callable_obj): + """ + Returns an ArgSpec object for functions, staticmethods, instance + methods, classmethods and partials. + + Note that the args list for instance and class methods are those as + seen by the user. In other words, the first argument with is + conventionally called 'self' or 'cls' is omitted in these cases. + """ + if inspect.isfunction(callable_obj): # functions and staticmethods + return inspect.getargspec(callable_obj) + elif isinstance(callable_obj, partial): # partials + arglen = len(callable_obj.args) + spec = inspect.getargspec(callable_obj.func) + args = [arg for arg in spec.args[arglen:] if arg not in callable_obj.keywords] + elif inspect.ismethod(callable_obj): # instance and class methods + spec = inspect.getargspec(callable_obj) + args = spec.args[1:] + else: # callable objects + return argspec(callable_obj.__call__) + + return inspect.ArgSpec(args = args, + varargs = spec.varargs, + keywords = spec.keywords, + defaults = spec.defaults) + def process_ellipses(obj, key, vdim_selection=False): """ Helper function to pad a __getitem__ key with the right number of From bbbcf80f280347cdbf340557ebe2281dad0058ec Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 15:39:38 +0100 Subject: [PATCH 02/24] Added Callable.argspec property --- holoviews/core/spaces.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/holoviews/core/spaces.py b/holoviews/core/spaces.py index 33e9b0cb52..b7db6a6526 100644 --- a/holoviews/core/spaces.py +++ b/holoviews/core/spaces.py @@ -442,6 +442,10 @@ def __init__(self, callable, **params): super(Callable, self).__init__(callable=callable, **params) self._memoized = {} + @property + def argspec(self): + return util.argspec(self.callable) + def __call__(self, *args, **kwargs): inputs = [i for i in self.inputs if isinstance(i, DynamicMap)] streams = [] From ce4b28409e717bc6c70eed03ac843aa0f3ac0672 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 15:40:26 +0100 Subject: [PATCH 03/24] Callable now promotes posargs to kwargs if possible --- holoviews/core/spaces.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/holoviews/core/spaces.py b/holoviews/core/spaces.py index b7db6a6526..dec8c32b5f 100644 --- a/holoviews/core/spaces.py +++ b/holoviews/core/spaces.py @@ -460,6 +460,22 @@ def __call__(self, *args, **kwargs): if memoize and hashed_key in self._memoized: return self._memoized[hashed_key] + if self.argspec.varargs is not None: + # Missing information on positional argument names, cannot promote to keywords + pass + elif len(args) != 0: # Turn positional arguments into keyword arguments + pos_kwargs = {k:v for k,v in zip(self.argspec.args, args)} + ignored = range(len(self.argspec.args),len(args)) + if len(ignored): + self.warning('Ignoring extra positional argument %s' + % ', '.join('%s' % i for i in ignored)) + clashes = set(pos_kwargs.keys()) & set(kwargs.keys()) + if clashes: + self.warning('Positional arguments %r overriden by keywords' + % list(clashes)) + args, kwargs = (), dict(pos_kwargs, **kwargs) + + ret = self.callable(*args, **kwargs) if hashed_key is not None: self._memoized = {hashed_key : ret} From edb26069f54e345c19b8ba9e21d3c998e329f9e5 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 15:41:26 +0100 Subject: [PATCH 04/24] Added assertContains method to MockLoggingHandler --- tests/__init__.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index 57f40b7072..b1d85b9cfa 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -27,6 +27,8 @@ class MockLoggingHandler(logging.Handler): def __init__(self, *args, **kwargs): self.messages = {'DEBUG': [], 'INFO': [], 'WARNING': [], 'ERROR': [], 'CRITICAL': [], 'VERBOSE':[]} + self.param_methods = {'WARNING':'param.warning()', 'INFO':'param.message()', + 'VERBOSE':'param.verbose()', 'DEBUG':'param.debug()'} super(MockLoggingHandler, self).__init__(*args, **kwargs) def emit(self, record): @@ -54,18 +56,30 @@ def assertEndsWith(self, level, substring): Assert that the last line captured at the given level ends with a particular substring. """ - methods = {'WARNING':'param.warning()', 'INFO':'param.message()', - 'VERBOSE':'param.verbose()', 'DEBUG':'param.debug()'} msg='\n\n{method}: {last_line}\ndoes not end with:\n{substring}' last_line = self.tail(level, n=1) if len(last_line) == 0: - raise AssertionError('Missing {method} output: {repr(substring)}'.format( - method=methods[level], substring=repr(substring))) + raise AssertionError('Missing {method} output: {substring}'.format( + method=self.param_methods[level], substring=repr(substring))) if not last_line[0].endswith(substring): - raise AssertionError(msg.format(method=methods[level], + raise AssertionError(msg.format(method=self.param_methods[level], last_line=repr(last_line[0]), substring=repr(substring))) + def assertContains(self, level, substring): + """ + Assert that the last line captured at the given level contains a + particular substring. + """ + msg='\n\n{method}: {last_line}\ndoes not contain:\n{substring}' + last_line = self.tail(level, n=1) + if len(last_line) == 0: + raise AssertionError('Missing {method} output: {substring}'.format( + method=self.param_methods[level], substring=repr(substring))) + if substring not in last_line[0]: + raise AssertionError(msg.format(method=self.param_methods[level], + last_line=repr(last_line[0]), + substring=repr(substring))) class LoggingComparisonTestCase(ComparisonTestCase): From 2be3cbb58183635f030a8b3507a649858d7d6573 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 15:43:00 +0100 Subject: [PATCH 05/24] Added 18 Callable unit tests in tests/testcallable.py --- tests/testcallable.py | 119 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/testcallable.py diff --git a/tests/testcallable.py b/tests/testcallable.py new file mode 100644 index 0000000000..50bd5934d3 --- /dev/null +++ b/tests/testcallable.py @@ -0,0 +1,119 @@ +# -*- coding: utf-8 -*- +""" +Unit tests of the Callable object that wraps user callbacks +""" +import param +from holoviews.element.comparison import ComparisonTestCase +from holoviews.core.spaces import Callable +from functools import partial + +from . import LoggingComparisonTestCase + +class CallableClass(object): + + def __call__(self, *testargs): + return sum(testargs) + + +class ParamFunc(param.ParameterizedFunction): + + a = param.Integer(default=1) + b = param.Number(default=1) + + def __call__(self, **params): + p = param.ParamOverrides(self, params) + return p.a * p.b + + +class TestSimpleCallableInvocation(LoggingComparisonTestCase): + + def setUp(self): + super(TestSimpleCallableInvocation, self).setUp() + + def test_callable_fn(self): + def callback(x): return x + self.assertEqual(Callable(callback)(3), 3) + + def test_callable_lambda(self): + self.assertEqual(Callable(lambda x,y: x+y)(3,5), 8) + + def test_callable_lambda_extras(self): + substr = "Ignoring extra positional argument" + self.assertEqual(Callable(lambda x,y: x+y)(3,5,10), 8) + self.log_handler.assertContains('WARNING', substr) + + def test_callable_lambda_extras_kwargs(self): + substr = "['x'] overriden by keywords" + self.assertEqual(Callable(lambda x,y: x+y)(3,5,x=10), 15) + self.log_handler.assertEndsWith('WARNING', substr) + + def test_callable_partial(self): + self.assertEqual(Callable(partial(lambda x,y: x+y,x=4))(5), 9) + + def test_callable_class(self): + self.assertEqual(Callable(CallableClass())(1,2,3,4), 10) + + def test_callable_paramfunc(self): + self.assertEqual(Callable(ParamFunc)(a=3,b=5), 15) + + +class TestCallableArgspec(ComparisonTestCase): + + def test_callable_fn_argspec(self): + def callback(x): return x + self.assertEqual(Callable(callback).argspec.args, ['x']) + self.assertEqual(Callable(callback).argspec.keywords, None) + + def test_callable_lambda_argspec(self): + self.assertEqual(Callable(lambda x,y: x+y).argspec.args, ['x','y']) + self.assertEqual(Callable(lambda x,y: x+y).argspec.keywords, None) + + def test_callable_partial_argspec(self): + self.assertEqual(Callable(partial(lambda x,y: x+y,x=4)).argspec.args, ['y']) + self.assertEqual(Callable(partial(lambda x,y: x+y,x=4)).argspec.keywords, None) + + def test_callable_class_argspec(self): + self.assertEqual(Callable(CallableClass()).argspec.args, []) + self.assertEqual(Callable(CallableClass()).argspec.keywords, None) + self.assertEqual(Callable(CallableClass()).argspec.varargs, 'testargs') + + def test_callable_paramfunc_argspec(self): + self.assertEqual(Callable(ParamFunc).argspec.args, []) + self.assertEqual(Callable(ParamFunc).argspec.keywords, 'params') + self.assertEqual(Callable(ParamFunc).argspec.varargs, None) + + +class TestKwargCallableInvocation(ComparisonTestCase): + """ + Test invocation of Callable with kwargs, even for callbacks with + positional arguments. + """ + + def test_callable_fn(self): + def callback(x): return x + self.assertEqual(Callable(callback)(x=3), 3) + + def test_callable_lambda(self): + self.assertEqual(Callable(lambda x,y: x+y)(x=3,y=5), 8) + + def test_callable_partial(self): + self.assertEqual(Callable(partial(lambda x,y: x+y,x=4))(y=5), 9) + + def test_callable_paramfunc(self): + self.assertEqual(Callable(ParamFunc)(a=3,b=5), 15) + + +class TestMixedCallableInvocation(ComparisonTestCase): + """ + Test mixed invocation of Callable with kwargs. + """ + + def test_callable_mixed_1(self): + def mixed_example(a,b, c=10, d=20): + return a+b+c+d + self.assertEqual(Callable(mixed_example)(a=3,b=5), 38) + + def test_callable_mixed_2(self): + def mixed_example(a,b, c=10, d=20): + return a+b+c+d + self.assertEqual(Callable(mixed_example)(3,5,5), 33) From 51a10166c1f671030d5ae0bec4c95d031d689a61 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 16:15:52 +0100 Subject: [PATCH 06/24] Fixed typo in docstring of argspec utility --- holoviews/core/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 8401d18b20..2c896a7861 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -108,7 +108,7 @@ def argspec(callable_obj): methods, classmethods and partials. Note that the args list for instance and class methods are those as - seen by the user. In other words, the first argument with is + seen by the user. In other words, the first argument which is conventionally called 'self' or 'cls' is omitted in these cases. """ if inspect.isfunction(callable_obj): # functions and staticmethods From a60ec5ca1685e1842a8d419a26a43d67056b75c2 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 16:41:20 +0100 Subject: [PATCH 07/24] Fixed py3 handling of ParameterizedFunction.__call__ in argspec --- holoviews/core/util.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 2c896a7861..72e561112e 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -111,7 +111,12 @@ def argspec(callable_obj): seen by the user. In other words, the first argument which is conventionally called 'self' or 'cls' is omitted in these cases. """ - if inspect.isfunction(callable_obj): # functions and staticmethods + if (isinstance(callable_obj, type) + and issubclass(callable_obj, param.ParameterizedFunction)): + # Parameterized function.__call__ considered function in py3 but not py2 + spec = inspect.getargspec(callable_obj.__call__) + args=spec.args[1:] + elif inspect.isfunction(callable_obj): # functions and staticmethods return inspect.getargspec(callable_obj) elif isinstance(callable_obj, partial): # partials arglen = len(callable_obj.args) From c05d369a3f6d78632f1fa718f9272ecae456862e Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 17:00:52 +0100 Subject: [PATCH 08/24] Added Callable tests of parameterized function instances --- tests/testcallable.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/testcallable.py b/tests/testcallable.py index 50bd5934d3..6bcc07b57b 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -20,9 +20,9 @@ class ParamFunc(param.ParameterizedFunction): a = param.Integer(default=1) b = param.Number(default=1) - def __call__(self, **params): + def __call__(self, a, **params): p = param.ParamOverrides(self, params) - return p.a * p.b + return a * p.b class TestSimpleCallableInvocation(LoggingComparisonTestCase): @@ -54,8 +54,10 @@ def test_callable_class(self): self.assertEqual(Callable(CallableClass())(1,2,3,4), 10) def test_callable_paramfunc(self): - self.assertEqual(Callable(ParamFunc)(a=3,b=5), 15) + self.assertEqual(Callable(ParamFunc)(3,b=5), 15) + def test_callable_paramfunc_instance(self): + self.assertEqual(Callable(ParamFunc.instance())(3,b=5), 15) class TestCallableArgspec(ComparisonTestCase): @@ -78,10 +80,14 @@ def test_callable_class_argspec(self): self.assertEqual(Callable(CallableClass()).argspec.varargs, 'testargs') def test_callable_paramfunc_argspec(self): - self.assertEqual(Callable(ParamFunc).argspec.args, []) + self.assertEqual(Callable(ParamFunc).argspec.args, ['a']) self.assertEqual(Callable(ParamFunc).argspec.keywords, 'params') self.assertEqual(Callable(ParamFunc).argspec.varargs, None) + def test_callable_paramfunc_instance_argspec(self): + self.assertEqual(Callable(ParamFunc.instance()).argspec.args, ['a']) + self.assertEqual(Callable(ParamFunc.instance()).argspec.keywords, 'params') + self.assertEqual(Callable(ParamFunc.instance()).argspec.varargs, None) class TestKwargCallableInvocation(ComparisonTestCase): """ From c74f3ea6cb765d2690ae75caece7ac3967435146 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 18:06:56 +0100 Subject: [PATCH 09/24] Added validate_dynamic_argspec utility to core.util --- holoviews/core/util.py | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 72e561112e..bcc6a73873 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -133,6 +133,52 @@ def argspec(callable_obj): keywords = spec.keywords, defaults = spec.defaults) + + +def validate_dynamic_argspec(argspec, kdims, streams): + """ + Utility used by DynamicMap to ensure the supplied callback has an + appropriate signature. + + If validation succeeds, returns a list of strings to be zipped with + the positional arguments i.e kdim values. The zipped values can then + be merged with the stream values to pass everything to the Callable + as keywords. + + If the callbacks use *args, None is returned to indicate that kdim + values must be passed to the Callable by position. In this + situation, Callable passes *args and **kwargs directly to the + callback. + + If the callback doesn't use **kwargs, the accepted keywords are + validated against the stream parameter names. + """ + + kdims = [kdim.name for kdim in kdims] + stream_params = stream_parameters(streams) + defaults = argspec.defaults if argspec.defaults else [] + posargs = argspec.args[:-len(defaults)] + kwargs = argspec.args[-len(defaults):] + + if argspec.keywords is None: + unassigned_streams = set(stream_params) - set(kwargs) + if unassigned_streams: + raise KeyError('Callable missing keywords to accept %s stream parameters' + % ', '.join(unassigned_streams)) + + if posargs == []: # No posargs, stream kwargs already validated + return [] + if set(kdims) == set(posargs): # Posargs match, can all be passed as kwargs + return kdims + elif len(posargs) == kdims: # Posargs match kdims length, supplying names + return posargs + elif argspec.varargs: # Posargs missing, passed to Callable directly + return None + else: + raise Exception('Supplied callback signature does not accommodate ' + 'required kdims and stream parameters') + + def process_ellipses(obj, key, vdim_selection=False): """ Helper function to pad a __getitem__ key with the right number of From 3f3655d6beddd646ba9db28138428620dc69a0f9 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 18:09:38 +0100 Subject: [PATCH 10/24] Validated callback signature in DynamicMap constructor --- holoviews/core/spaces.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/holoviews/core/spaces.py b/holoviews/core/spaces.py index dec8c32b5f..9e4c2b8028 100644 --- a/holoviews/core/spaces.py +++ b/holoviews/core/spaces.py @@ -556,6 +556,9 @@ def __init__(self, callback, initial_items=None, **params): callback = Callable(callback) super(DynamicMap, self).__init__(initial_items, callback=callback, **params) + self._posarg_keys = util.validate_dynamic_argspec(self.callback.argspec, + self.kdims, + self.streams) # Set source to self if not already specified for stream in self.streams: if stream.source is None: From 25c91b69cf94933a1c6001328890393a87696dd2 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 18:10:32 +0100 Subject: [PATCH 11/24] Generalized how DynamicMap._execute_callback invokes Callable --- holoviews/core/spaces.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/holoviews/core/spaces.py b/holoviews/core/spaces.py index 9e4c2b8028..646051cf51 100644 --- a/holoviews/core/spaces.py +++ b/holoviews/core/spaces.py @@ -648,7 +648,14 @@ def _execute_callback(self, *args): flattened = [(k,v) for kws in kwarg_items for (k,v) in kws if k not in kdims] with dynamicmap_memoization(self.callback, self.streams): - retval = self.callback(*args, **dict(flattened)) + + if self._posarg_keys: + kwargs = dict(flattened, **dict(zip(self._posarg_keys, args))) + args = () + else: + kwargs = dict(flattened) + + retval = self.callback(*args, **kwargs) return self._style(retval) From 50022dffcf2c3a176e9cb0e407d162a23096f08b Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 18:37:28 +0100 Subject: [PATCH 12/24] Computing *args and **kwargs outside of memoization context manager --- holoviews/core/spaces.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/holoviews/core/spaces.py b/holoviews/core/spaces.py index 646051cf51..d0688d3ef7 100644 --- a/holoviews/core/spaces.py +++ b/holoviews/core/spaces.py @@ -647,14 +647,14 @@ def _execute_callback(self, *args): kwarg_items = [s.contents.items() for s in self.streams] flattened = [(k,v) for kws in kwarg_items for (k,v) in kws if k not in kdims] - with dynamicmap_memoization(self.callback, self.streams): - if self._posarg_keys: - kwargs = dict(flattened, **dict(zip(self._posarg_keys, args))) - args = () - else: - kwargs = dict(flattened) + if self._posarg_keys: + kwargs = dict(flattened, **dict(zip(self._posarg_keys, args))) + args = () + else: + kwargs = dict(flattened) + with dynamicmap_memoization(self.callback, self.streams): retval = self.callback(*args, **kwargs) return self._style(retval) From 3d5be47b1fd9fbbe8e006f894225649fd0cf3eb7 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 19:13:55 +0100 Subject: [PATCH 13/24] Fixed invalid unit test definitions in testdynamic.py --- tests/testdynamic.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testdynamic.py b/tests/testdynamic.py index 14705405d4..d6f842aeb2 100644 --- a/tests/testdynamic.py +++ b/tests/testdynamic.py @@ -187,16 +187,16 @@ def fn2(x, y): def test_dynamic_event_renaming_valid(self): - def fn(x, y): - return Scatter([(x, y)]) + def fn(x1, y1): + return Scatter([(x1, y1)]) xy = PositionXY(rename={'x':'x1','y':'y1'}) dmap = DynamicMap(fn, kdims=[], streams=[xy]) dmap.event(x1=1, y1=2) def test_dynamic_event_renaming_invalid(self): - def fn(x, y): - return Scatter([(x, y)]) + def fn(x1, y1): + return Scatter([(x1, y1)]) xy = PositionXY(rename={'x':'x1','y':'y1'}) dmap = DynamicMap(fn, kdims=[], streams=[xy]) From d5f060ec11e86174d435827aca6eb2553b10b213 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 19:14:50 +0100 Subject: [PATCH 14/24] validate_dynamic_argspec now handles positional stream arguments --- holoviews/core/util.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index bcc6a73873..66a2e3a795 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -161,7 +161,7 @@ def validate_dynamic_argspec(argspec, kdims, streams): kwargs = argspec.args[-len(defaults):] if argspec.keywords is None: - unassigned_streams = set(stream_params) - set(kwargs) + unassigned_streams = set(stream_params) - set(argspec.args) if unassigned_streams: raise KeyError('Callable missing keywords to accept %s stream parameters' % ', '.join(unassigned_streams)) @@ -174,9 +174,13 @@ def validate_dynamic_argspec(argspec, kdims, streams): return posargs elif argspec.varargs: # Posargs missing, passed to Callable directly return None - else: + + # Handle positional arguments matching stream parameter names + stream_posargs = [arg for arg in posargs if arg in stream_params] + if len(stream_posargs) != len(posargs): raise Exception('Supplied callback signature does not accommodate ' 'required kdims and stream parameters') + return stream_posargs def process_ellipses(obj, key, vdim_selection=False): From 32b3811d9282f54d593727025ff4ee86c87fc969 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 20:04:57 +0100 Subject: [PATCH 15/24] validate_dynamic_argspec now supports streams as positional args --- holoviews/core/util.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 66a2e3a795..44759f8a03 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -157,7 +157,9 @@ def validate_dynamic_argspec(argspec, kdims, streams): kdims = [kdim.name for kdim in kdims] stream_params = stream_parameters(streams) defaults = argspec.defaults if argspec.defaults else [] - posargs = argspec.args[:-len(defaults)] + all_posargs = argspec.args[:-len(defaults)] if defaults else argspec.args + # Filter out any posargs for streams + posargs = [arg for arg in all_posargs if arg not in stream_params] kwargs = argspec.args[-len(defaults):] if argspec.keywords is None: @@ -170,17 +172,11 @@ def validate_dynamic_argspec(argspec, kdims, streams): return [] if set(kdims) == set(posargs): # Posargs match, can all be passed as kwargs return kdims - elif len(posargs) == kdims: # Posargs match kdims length, supplying names + elif len(posargs) == kdims: # Posargs match kdims length, supplying names return posargs elif argspec.varargs: # Posargs missing, passed to Callable directly return None - - # Handle positional arguments matching stream parameter names - stream_posargs = [arg for arg in posargs if arg in stream_params] - if len(stream_posargs) != len(posargs): - raise Exception('Supplied callback signature does not accommodate ' - 'required kdims and stream parameters') - return stream_posargs + return None # Use strict invocation def process_ellipses(obj, key, vdim_selection=False): From d5063f9d291f92c90ecc0bdd8e5b29bfb7580625 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 20:13:44 +0100 Subject: [PATCH 16/24] Fixed bug in validate_dynamic_argspec utility --- holoviews/core/util.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 44759f8a03..bdd7c5cb8b 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -172,11 +172,14 @@ def validate_dynamic_argspec(argspec, kdims, streams): return [] if set(kdims) == set(posargs): # Posargs match, can all be passed as kwargs return kdims - elif len(posargs) == kdims: # Posargs match kdims length, supplying names + elif len(posargs) == len(kdims): # Posargs match kdims length, supplying names return posargs elif argspec.varargs: # Posargs missing, passed to Callable directly return None - return None # Use strict invocation + else: + raise Exception('Callback positional arguments {posargs} do not accommodate ' + 'required kdims {kdims}'.format(posargs=posargs, kdims=kdims)) + def process_ellipses(obj, key, vdim_selection=False): From f5efe4e064d0ec2d18c10abe5e7d69c8c132c593 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 20:47:35 +0100 Subject: [PATCH 17/24] validate_dynamic_argspec now only raises KeyError exceptions --- holoviews/core/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index bdd7c5cb8b..7c312f9a34 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -177,8 +177,8 @@ def validate_dynamic_argspec(argspec, kdims, streams): elif argspec.varargs: # Posargs missing, passed to Callable directly return None else: - raise Exception('Callback positional arguments {posargs} do not accommodate ' - 'required kdims {kdims}'.format(posargs=posargs, kdims=kdims)) + raise KeyError('Callback positional arguments {posargs} do not accommodate ' + 'required kdims {kdims}'.format(posargs=posargs, kdims=kdims)) From 0fae1e5f15d0cc59afbd044a0dbf847338388bff Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 20:48:28 +0100 Subject: [PATCH 18/24] Added 10 unit tests for different allowed Callable signatures --- tests/testcallable.py | 109 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/tests/testcallable.py b/tests/testcallable.py index 6bcc07b57b..15a58da509 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -1,10 +1,13 @@ # -*- coding: utf-8 -*- """ -Unit tests of the Callable object that wraps user callbacks +Unit tests of the Callable object that wraps user callbacks. Also test +how DynamicMap validates and invokes Callable based on its signature. """ import param from holoviews.element.comparison import ComparisonTestCase -from holoviews.core.spaces import Callable +from holoviews.element import Scatter +from holoviews import streams +from holoviews.core.spaces import Callable, DynamicMap from functools import partial from . import LoggingComparisonTestCase @@ -59,6 +62,7 @@ def test_callable_paramfunc(self): def test_callable_paramfunc_instance(self): self.assertEqual(Callable(ParamFunc.instance())(3,b=5), 15) + class TestCallableArgspec(ComparisonTestCase): def test_callable_fn_argspec(self): @@ -89,6 +93,7 @@ def test_callable_paramfunc_instance_argspec(self): self.assertEqual(Callable(ParamFunc.instance()).argspec.keywords, 'params') self.assertEqual(Callable(ParamFunc.instance()).argspec.varargs, None) + class TestKwargCallableInvocation(ComparisonTestCase): """ Test invocation of Callable with kwargs, even for callbacks with @@ -123,3 +128,103 @@ def test_callable_mixed_2(self): def mixed_example(a,b, c=10, d=20): return a+b+c+d self.assertEqual(Callable(mixed_example)(3,5,5), 33) + + +class TestDynamicMapInvocation(ComparisonTestCase): + """ + Test that DynamicMap passes kdims and stream parameters correctly to + Callables. + """ + + def test_dynamic_kdims_only(self): + def fn(A,B): + return Scatter([(A,2)], label=A) + + dmap = DynamicMap(fn, kdims=['A','B'], sampled=True) + self.assertEqual(dmap['Test', 1], Scatter([(1, 2)], label='Test')) + + def test_dynamic_kdims_only_invalid(self): + def fn(A,B): + return Scatter([(A,2)], label=A) + + regexp="Callback positional arguments (.+?) do not accommodate required kdims (.+?)" + with self.assertRaisesRegexp(KeyError, regexp): + dmap = DynamicMap(fn, kdims=['A'], sampled=True) + + + def test_dynamic_kdims_args_only(self): + def fn(*args): + (A,B) = args + return Scatter([(A,2)], label=A) + + dmap = DynamicMap(fn, kdims=['A','B'], sampled=True) + self.assertEqual(dmap['Test', 1], Scatter([(1, 2)], label='Test')) + + + def test_dynamic_streams_only_kwargs(self): + def fn(x=1, y=2): + return Scatter([(x,y)], label='default') + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=[], streams=[xy], sampled=True) + self.assertEqual(dmap[:], Scatter([(1, 2)], label='default')) + + + def test_dynamic_streams_only_keywords(self): + def fn(**kwargs): + return Scatter([(kwargs['x'],kwargs['y'])], label='default') + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=[], streams=[xy], sampled=True) + self.assertEqual(dmap[:], Scatter([(1, 2)], label='default')) + + + def test_dynamic_split_kdims_and_streams(self): + # Corresponds to the old style of kdims as posargs and streams + # as kwargs + def fn(A, x=1, y=2): + return Scatter([(x,y)], label=A) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + + def test_dynamic_split_args_and_kwargs(self): + # Corresponds to the old style of kdims as posargs and streams + # as kwargs, captured as *args and **kwargs + def fn(*args, **kwargs): + return Scatter([(kwargs['x'],kwargs['y'])], label=args[0]) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + + def test_dynamic_all_keywords(self): + def fn(A='default', x=1, y=2): + return Scatter([(x,y)], label=A) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + + def test_dynamic_keywords_and_kwargs(self): + def fn(A='default', x=1, y=2, **kws): + return Scatter([(x,y)], label=A) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + + def test_dynamic_mixed_kwargs(self): + def fn(x, A, y): + return Scatter([(x, y)], label=A) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + From 09fec6de5305043a1cfc0e6abe4d718b0637a586 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 20:55:35 +0100 Subject: [PATCH 19/24] Added unit test for when kdim posargs have different names --- tests/testcallable.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/testcallable.py b/tests/testcallable.py index 15a58da509..b7c599f445 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -190,6 +190,17 @@ def fn(A, x=1, y=2): self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + def test_dynamic_split_mismatched_kdims_and_streams(self): + # Corresponds to the old style of kdims as posargs and streams + # as kwargs. Positional arg names don't have to match + def fn(B, x=1, y=2): + return Scatter([(x,y)], label=B) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + + def test_dynamic_split_args_and_kwargs(self): # Corresponds to the old style of kdims as posargs and streams # as kwargs, captured as *args and **kwargs From ebc8ffa97c8e807a12b4a10d7a932231f772a959 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 21:45:03 +0100 Subject: [PATCH 20/24] Improved validation in validate_dynamic_argspec utility --- holoviews/core/util.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 7c312f9a34..7a12c02c7b 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -168,7 +168,7 @@ def validate_dynamic_argspec(argspec, kdims, streams): raise KeyError('Callable missing keywords to accept %s stream parameters' % ', '.join(unassigned_streams)) - if posargs == []: # No posargs, stream kwargs already validated + if kdims == []: # Can be no posargs, stream kwargs already validated return [] if set(kdims) == set(posargs): # Posargs match, can all be passed as kwargs return kdims @@ -176,9 +176,16 @@ def validate_dynamic_argspec(argspec, kdims, streams): return posargs elif argspec.varargs: # Posargs missing, passed to Callable directly return None + elif set(posargs) - set(kdims): + raise KeyError('Callable accepts more positional arguments {posargs} ' + 'than there are key dimensions {kdims}'.format(posargs=posargs, + kdims=kdims)) + elif set(kdims).issubset(set(kwargs)): # Key dims can be supplied by keyword + return kdims else: - raise KeyError('Callback positional arguments {posargs} do not accommodate ' - 'required kdims {kdims}'.format(posargs=posargs, kdims=kdims)) + raise KeyError('Callback signature over {names} do not accommodate ' + 'required kdims {kdims}'.format(names=list(set(posargs+kwargs)), + kdims=kdims)) From fccdab4554c27f1528cce8d2371de8510335b361 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 21:46:19 +0100 Subject: [PATCH 21/24] Formatting fixes to comments --- holoviews/core/util.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 7a12c02c7b..d0651a2f98 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -168,13 +168,13 @@ def validate_dynamic_argspec(argspec, kdims, streams): raise KeyError('Callable missing keywords to accept %s stream parameters' % ', '.join(unassigned_streams)) - if kdims == []: # Can be no posargs, stream kwargs already validated + if kdims == []: # Can be no posargs, stream kwargs already validated return [] - if set(kdims) == set(posargs): # Posargs match, can all be passed as kwargs + if set(kdims) == set(posargs): # Posargs match exactly, can all be passed as kwargs return kdims - elif len(posargs) == len(kdims): # Posargs match kdims length, supplying names + elif len(posargs) == len(kdims): # Posargs match kdims length, supplying names return posargs - elif argspec.varargs: # Posargs missing, passed to Callable directly + elif argspec.varargs: # Posargs missing, passed to Callable directly return None elif set(posargs) - set(kdims): raise KeyError('Callable accepts more positional arguments {posargs} ' From 5ec7a2beac0a0e6b1a506a1944ae60e7686954a4 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 21:47:38 +0100 Subject: [PATCH 22/24] Added unit tests for invalid call conditions --- tests/testcallable.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/testcallable.py b/tests/testcallable.py index b7c599f445..977f33c6bc 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -147,7 +147,7 @@ def test_dynamic_kdims_only_invalid(self): def fn(A,B): return Scatter([(A,2)], label=A) - regexp="Callback positional arguments (.+?) do not accommodate required kdims (.+?)" + regexp="Callable accepts more positional arguments (.+?) than there are key dimensions (.+?)" with self.assertRaisesRegexp(KeyError, regexp): dmap = DynamicMap(fn, kdims=['A'], sampled=True) @@ -189,8 +189,18 @@ def fn(A, x=1, y=2): dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + def test_dynamic_split_kdims_and_streams_invalid(self): + # Corresponds to the old style of kdims as posargs and streams + # as kwargs. Positional arg names don't have to match + def fn(x=1, y=2, B='default'): + return Scatter([(x,y)], label=B) + + xy = streams.PositionXY(x=1, y=2) + regexp = "Callback signature over (.+?) do not accommodate required kdims" + with self.assertRaisesRegexp(KeyError, regexp): + DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) - def test_dynamic_split_mismatched_kdims_and_streams(self): + def test_dynamic_split_mismatched_kdims_example1(self): # Corresponds to the old style of kdims as posargs and streams # as kwargs. Positional arg names don't have to match def fn(B, x=1, y=2): @@ -200,6 +210,16 @@ def fn(B, x=1, y=2): dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + def test_dynamic_split_mismatched_kdims_example2(self): + # Corresponds to the old style of kdims as posargs and streams + # as kwargs. Positional arg names don't have to match and the + # stream parameters can be passed by position + def fn(x, y, B): + return Scatter([(x,y)], label=B) + + xy = streams.PositionXY(x=1, y=2) + dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) + self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) def test_dynamic_split_args_and_kwargs(self): # Corresponds to the old style of kdims as posargs and streams From 111e3334cbcf326b09e600065330d088bb511dc8 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 21:56:31 +0100 Subject: [PATCH 23/24] Unmatched positional kdims must be at the start of the signature --- holoviews/core/util.py | 4 ++++ tests/testcallable.py | 13 ++++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index d0651a2f98..597acbd7a9 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -173,6 +173,10 @@ def validate_dynamic_argspec(argspec, kdims, streams): if set(kdims) == set(posargs): # Posargs match exactly, can all be passed as kwargs return kdims elif len(posargs) == len(kdims): # Posargs match kdims length, supplying names + if argspec.args[:len(kdims)] != posargs: + raise KeyError('Unmatched positional kdim arguments only ' + 'allowed at the start of the signature') + return posargs elif argspec.varargs: # Posargs missing, passed to Callable directly return None diff --git a/tests/testcallable.py b/tests/testcallable.py index 977f33c6bc..562f12535f 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -200,7 +200,7 @@ def fn(x=1, y=2, B='default'): with self.assertRaisesRegexp(KeyError, regexp): DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) - def test_dynamic_split_mismatched_kdims_example1(self): + def test_dynamic_split_mismatched_kdims(self): # Corresponds to the old style of kdims as posargs and streams # as kwargs. Positional arg names don't have to match def fn(B, x=1, y=2): @@ -210,16 +210,19 @@ def fn(B, x=1, y=2): dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) - def test_dynamic_split_mismatched_kdims_example2(self): + def test_dynamic_split_mismatched_kdims_invalid(self): # Corresponds to the old style of kdims as posargs and streams # as kwargs. Positional arg names don't have to match and the - # stream parameters can be passed by position + # stream parameters can be passed by position but *only* if they + # come first def fn(x, y, B): return Scatter([(x,y)], label=B) xy = streams.PositionXY(x=1, y=2) - dmap = DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) - self.assertEqual(dmap['Test'], Scatter([(1, 2)], label='Test')) + regexp = ("Unmatched positional kdim arguments only allowed " + "at the start of the signature") + with self.assertRaisesRegexp(KeyError, regexp): + DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True) def test_dynamic_split_args_and_kwargs(self): # Corresponds to the old style of kdims as posargs and streams From f5c370cc7426ff19adedca3937dbc381035b4e72 Mon Sep 17 00:00:00 2001 From: jlstevens Date: Mon, 10 Apr 2017 21:58:20 +0100 Subject: [PATCH 24/24] Fixed exception message --- holoviews/core/util.py | 2 +- tests/testcallable.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/holoviews/core/util.py b/holoviews/core/util.py index 597acbd7a9..19055216e4 100644 --- a/holoviews/core/util.py +++ b/holoviews/core/util.py @@ -187,7 +187,7 @@ def validate_dynamic_argspec(argspec, kdims, streams): elif set(kdims).issubset(set(kwargs)): # Key dims can be supplied by keyword return kdims else: - raise KeyError('Callback signature over {names} do not accommodate ' + raise KeyError('Callback signature over {names} does not accommodate ' 'required kdims {kdims}'.format(names=list(set(posargs+kwargs)), kdims=kdims)) diff --git a/tests/testcallable.py b/tests/testcallable.py index 562f12535f..4fc9b0b43e 100644 --- a/tests/testcallable.py +++ b/tests/testcallable.py @@ -196,7 +196,7 @@ def fn(x=1, y=2, B='default'): return Scatter([(x,y)], label=B) xy = streams.PositionXY(x=1, y=2) - regexp = "Callback signature over (.+?) do not accommodate required kdims" + regexp = "Callback signature over (.+?) does not accommodate required kdims" with self.assertRaisesRegexp(KeyError, regexp): DynamicMap(fn, kdims=['A'], streams=[xy], sampled=True)