Skip to content

[WIP] Delay callback validation #644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 81 additions & 60 deletions dash/dash.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import warnings
import re
import logging
import pprint

from functools import wraps

Expand Down Expand Up @@ -165,6 +164,9 @@ def __init__(
# list of dependencies
self.callback_map = {}

# for delayed callback validation
self._callback_list = []

self._index_string = ''
self.index_string = index_string
self._meta_tags = meta_tags or []
Expand Down Expand Up @@ -641,6 +643,73 @@ def react(self, *args, **kwargs):
'Use `callback` instead. `callback` has a new syntax too, '
'so make sure to call `help(app.callback)` to learn more.')

def _validate_callbacks(self):
# Maps all registered Output components (including individual Outputs
# within multi-output callbacks) onto their associated callback
# function names)
output_map = collections.defaultdict(list)

for output, inputs, state, func_name in self._callback_list:
self._validate_callback(output, inputs, state)

outputs = output if isinstance(output, (list, tuple)) else [output]
for out in outputs:
output_map[out].append(func_name)

# Each Output should only have one callback function associated with it
duplicates = [(output, funcs) for output, funcs in output_map.items()
if len(funcs) > 1]
if not duplicates:
return

msg1 = ""
msg2 = ""
diff_callback = []
same_callback = []

def get_error_lines(dupes):
err_str = '{} in callback function{}: {}'
lines = []
for output, funcs in dupes:
func_names = ', '.join("'{}'".format(f) for f in funcs)
plural = 's' if len(funcs) > 1 else ''
lines.append(err_str.format(repr(output), plural, func_names))
return '\n'.join(lines)

for output, funcs in duplicates:
if len(set(funcs)) < len(funcs):
# Single Output used multiple times in multi output callback
fs = set(f for f in funcs if funcs.count(f) > 1)
same_callback.append((output, sorted(fs)))
if not all(funcs[0] == f for f in funcs):
# Output used across multiple callbacks
diff_callback.append((output, sorted(set(funcs))))

if diff_callback:
errors = get_error_lines(diff_callback)
msg1 = '''
The following outputs have been assigned to multiple callbacks.

{}

An output can only be associated with a single callback function.
Try combining your inputs and callback functions together into one
function.'''.format(errors).replace(' ', '')

if same_callback:
errors = get_error_lines(same_callback)
msg2 = '''
The following outputs occur multiple times in a single multi-output
callback:

{}

Multi-output callbacks cannot target the same output more than
once.'''.format(errors).replace(' ', '')

sep = '\n' if msg1 and msg2 else ''
raise exceptions.DuplicateCallbackOutput(msg1 + sep + msg2)

def _validate_callback(self, output, inputs, state):
# pylint: disable=too-many-branches
layout = self._cached_layout or self._layout_value()
Expand All @@ -660,19 +729,6 @@ def _validate_callback(self, output, inputs, state):
'Same output and input: {}'.format(bad)
)

if is_multi:
if len(set(output)) != len(output):
raise exceptions.DuplicateCallbackOutput(
'Same output was used in a'
' multi output callback!\n Duplicates:\n {}'.format(
',\n'.join(
k for k, v in
((str(x), output.count(x)) for x in output)
if v > 1
)
)
)

if (layout is None and
not self.config.first('suppress_callback_exceptions',
'supress_callback_exceptions')):
Expand Down Expand Up @@ -787,49 +843,6 @@ def _validate_callback(self, output, inputs, state):
'elements' if len(state) > 1 else 'element'
).replace(' ', ''))

callback_id = _create_callback_id(output)

callbacks = set(itertools.chain(*(
x[2:-2].split('...')
if x.startswith('..')
else [x]
for x in self.callback_map
)))
ns = {
'duplicates': set()
}
if is_multi:
def duplicate_check():
ns['duplicates'] = callbacks.intersection(
str(y) for y in output
)
return ns['duplicates']
else:
def duplicate_check():
return callback_id in callbacks
if duplicate_check():
if is_multi:
msg = '''
Multi output {} contains an `Output` object
that was already assigned.
Duplicates:
{}
'''.format(
callback_id,
pprint.pformat(ns['duplicates'])
)
else:
msg = '''
You have already assigned a callback to the output
with ID "{}" and property "{}". An output can only have
a single callback function. Try combining your inputs and
callback functions together into one function.
'''.format(
output.component_id,
output.component_property
).replace(' ', '')
raise exceptions.DuplicateCallbackOutput(msg)

def _validate_callback_output(self, output_value, output):
valid = [str, dict, int, float, type(None), Component]

Expand Down Expand Up @@ -947,8 +960,6 @@ def _validate_value(val, index=None):
# relationships
# pylint: disable=dangerous-default-value
def callback(self, output, inputs=[], state=[]):
self._validate_callback(output, inputs, state)

callback_id = _create_callback_id(output)
multi = isinstance(output, (list, tuple))

Expand Down Expand Up @@ -1026,7 +1037,7 @@ def add_context(*args, **kwargs):
return jsonResponse

self.callback_map[callback_id]['callback'] = add_context

self._callback_list.append((output, inputs, state, func.__name__))
return add_context

return wrap_func
Expand Down Expand Up @@ -1375,6 +1386,7 @@ def run_server(self,
:param flask_run_options: Given to `Flask.run`
:return:
"""
self._setup()
debug = self.enable_dev_tools(
debug,
dev_tools_serve_dev_bundles,
Expand Down Expand Up @@ -1409,3 +1421,12 @@ def run_server(self,

self.server.run(port=port, debug=debug,
**flask_run_options)

def _setup(self):
"""Do setup required for both dev server and production WSGI server"""
self._validate_callbacks()

def get_wsgi_application(self, debug=False):
self._setup()
self.enable_dev_tools(debug)
return self.server
154 changes: 98 additions & 56 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,53 @@ def test_multi_output(self):
html.Div(id='output5')
])

@app.callback([Output('output1', 'children'), Output('output2', 'children')],
[Input('output-btn', 'n_clicks')],
[State('output-btn', 'n_clicks_timestamp')])
def on_click(n_clicks, n_clicks_timestamp):
if n_clicks is None:
raise PreventUpdate

return n_clicks, n_clicks_timestamp

self.startServer(app)
t = time.time()

btn = self.wait_for_element_by_id('output-btn')
btn.click()
time.sleep(1)

self.wait_for_text_to_equal('#output1', '1')
output2 = self.wait_for_element_by_css_selector('#output2')

self.assertGreater(int(output2.text), t)

def test_multi_output_duplicates(self):
app = dash.Dash(__name__)
app.scripts.config.serve_locally = True

app.layout = html.Div([
html.Button('OUTPUT', id='output-btn'),

html.Table([
html.Thead([
html.Tr([
html.Th('Output 1'),
html.Th('Output 2')
])
]),
html.Tbody([
html.Tr([html.Td(id='output1'), html.Td(id='output2')]),
])
]),

html.Div(id='output3'),
html.Div(id='output4'),
html.Div(id='output5')
])

bad_outputs = []

@app.callback([Output('output1', 'children'), Output('output2', 'children')],
[Input('output-btn', 'n_clicks')],
[State('output-btn', 'n_clicks_timestamp')])
Expand All @@ -599,63 +646,48 @@ def dummy_callback(n_clicks):
return 'Output 3: {}'.format(n_clicks)

# Test that a multi output can't be included in a single output
with self.assertRaises(DuplicateCallbackOutput) as context:
@app.callback(Output('output1', 'children'),
[Input('output-btn', 'n_clicks')])
def on_click_duplicate(n_clicks):
if n_clicks is None:
raise PreventUpdate
bad_outputs.append('output1.children')

return 'something else'
@app.callback(Output('output1', 'children'),
[Input('output-btn', 'n_clicks')])
def on_click_duplicate(n_clicks):
if n_clicks is None:
raise PreventUpdate

self.assertTrue('output1' in context.exception.args[0])
return 'something else'

# Test a multi output cannot contain a used single output
with self.assertRaises(DuplicateCallbackOutput) as context:
@app.callback([Output('output3', 'children'),
Output('output4', 'children')],
[Input('output-btn', 'n_clicks')])
def on_click_duplicate_multi(n_clicks):
if n_clicks is None:
raise PreventUpdate

return 'something else'

self.assertTrue('output3' in context.exception.args[0])
bad_outputs.append('output3.children')

with self.assertRaises(DuplicateCallbackOutput) as context:
@app.callback([Output('output5', 'children'),
Output('output5', 'children')],
[Input('output-btn', 'n_clicks')])
def on_click_same_output(n_clicks):
return n_clicks
@app.callback([Output('output3', 'children'),
Output('output4', 'children')],
[Input('output-btn', 'n_clicks')])
def on_click_duplicate_multi(n_clicks):
if n_clicks is None:
raise PreventUpdate

self.assertTrue('output5' in context.exception.args[0])
return 'something else'

with self.assertRaises(DuplicateCallbackOutput) as context:
@app.callback([Output('output1', 'children'),
Output('output5', 'children')],
[Input('output-btn', 'n_clicks')])
def overlapping_multi_output(n_clicks):
return n_clicks

self.assertTrue(
'{\'output1.children\'}' in context.exception.args[0]
or "set(['output1.children'])" in context.exception.args[0]
)
# Test multi output cannot include same output more than once
bad_outputs.append('output5.children')

self.startServer(app)
@app.callback([Output('output5', 'children'),
Output('output5', 'children')],
[Input('output-btn', 'n_clicks')])
def on_click_same_output(n_clicks):
return n_clicks

t = time.time()
@app.callback([Output('output1', 'children'),
Output('output5', 'children')],
[Input('output-btn', 'n_clicks')])
def overlapping_multi_output(n_clicks):
return n_clicks

btn = self.wait_for_element_by_id('output-btn')
btn.click()
time.sleep(1)
with self.assertRaises(DuplicateCallbackOutput) as context:
app._setup()

self.wait_for_text_to_equal('#output1', '1')
output2 = self.wait_for_element_by_css_selector('#output2')

self.assertGreater(int(output2.text), t)
for output in bad_outputs:
self.assertTrue(output in context.exception.args[0])

def test_with_custom_renderer(self):
app = dash.Dash(__name__)
Expand Down Expand Up @@ -880,24 +912,34 @@ def test_output_input_invalid_callback(self):
html.Div(id='out')
])

@app.callback(Output('input-output', 'children'),
[Input('input-output', 'children')])
def failure(children):
pass

with self.assertRaises(CallbackException) as context:
@app.callback(Output('input-output', 'children'),
[Input('input-output', 'children')])
def failure(children):
pass
app._setup()

self.assertEqual(
'Same output and input: input-output.children',
context.exception.args[0]
)

# Multi output version.
def test_multi_output_input_invalid_callback(self):
app = dash.Dash(__name__)
app.layout = html.Div([
html.Div('child', id='input-output'),
html.Div(id='out')
])

@app.callback([Output('out', 'children'),
Output('input-output', 'children')],
[Input('input-output', 'children')])
def failure2(children):
pass

with self.assertRaises(CallbackException) as context:
@app.callback([Output('out', 'children'),
Output('input-output', 'children')],
[Input('input-output', 'children')])
def failure2(children):
pass
app._setup()

self.assertEqual(
'Same output and input: input-output.children',
Expand Down