From 69dc289fe3d596ef8cf375dff5e5bf07316fd980 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Sat, 16 Jan 2021 20:34:43 -0500 Subject: [PATCH 1/2] more careful exception dedenting still dedent in the base class, but anywhere there could be multiline interpolated args, dedent before interpolation too. --- dash/_utils.py | 15 +++-- dash/_validate.py | 161 ++++++++++++++++++++++++---------------------- dash/resources.py | 17 +++-- 3 files changed, 105 insertions(+), 88 deletions(-) diff --git a/dash/_utils.py b/dash/_utils.py index 51c476c9c3..d073bf6e6d 100644 --- a/dash/_utils.py +++ b/dash/_utils.py @@ -65,8 +65,12 @@ def get_relative_path(requests_pathname, path): return requests_pathname elif not path.startswith("/"): raise exceptions.UnsupportedRelativePath( - "Paths that aren't prefixed with a leading / are not supported.\n" - + "You supplied: {}".format(path) + """ + Paths that aren't prefixed with a leading / are not supported. + You supplied: {} + """.format( + path + ) ) return "/".join([requests_pathname.rstrip("/"), path.lstrip("/")]) @@ -78,9 +82,10 @@ def strip_relative_path(requests_pathname, path): requests_pathname != "/" and not path.startswith(requests_pathname.rstrip("/")) ) or (requests_pathname == "/" and not path.startswith("/")): raise exceptions.UnsupportedRelativePath( - "Paths that aren't prefixed with a leading " - + "requests_pathname_prefix are not supported.\n" - + "You supplied: {} and requests_pathname_prefix was {}".format( + """ + Paths that aren't prefixed with requests_pathname_prefix are not supported. + You supplied: {} and requests_pathname_prefix was {} + """.format( path, requests_pathname ) ) diff --git a/dash/_validate.py b/dash/_validate.py index 76047242a6..d06856bc8c 100644 --- a/dash/_validate.py +++ b/dash/_validate.py @@ -1,5 +1,6 @@ import collections import re +from textwrap import dedent from .development.base_component import Component from . import exceptions @@ -15,26 +16,26 @@ def validate_callback(output, inputs, state, extra_args, types): if extra_args: if not isinstance(extra_args[0], (Output, Input, State)): raise exceptions.IncorrectTypeException( - """ - Callback arguments must be `Output`, `Input`, or `State` objects, - optionally wrapped in a list or tuple. We found (possibly after - unwrapping a list or tuple): - {} - """.format( - repr(extra_args[0]) - ) + dedent( + """ + Callback arguments must be `Output`, `Input`, or `State` objects, + optionally wrapped in a list or tuple. We found (possibly after + unwrapping a list or tuple): + {} + """ + ).format(repr(extra_args[0])) ) raise exceptions.IncorrectTypeException( - """ - In a callback definition, you must provide all Outputs first, - then all Inputs, then all States. After this item: - {} - we found this item next: - {} - """.format( - repr((outputs + inputs + state)[-1]), repr(extra_args[0]) - ) + dedent( + """ + In a callback definition, you must provide all Outputs first, + then all Inputs, then all States. After this item: + {} + we found this item next: + {} + """ + ).format(repr((outputs + inputs + state)[-1]), repr(extra_args[0])) ) for args in [outputs, inputs, state]: @@ -45,11 +46,11 @@ def validate_callback(output, inputs, state, extra_args, types): def validate_callback_arg(arg): if not isinstance(getattr(arg, "component_property", None), _strings): raise exceptions.IncorrectTypeException( - """ - component_property must be a string, found {!r} - """.format( - arg.component_property - ) + dedent( + """ + component_property must be a string, found {!r} + """ + ).format(arg.component_property) ) if hasattr(arg, "component_event"): @@ -68,11 +69,11 @@ def validate_callback_arg(arg): else: raise exceptions.IncorrectTypeException( - """ - component_id must be a string or dict, found {!r} - """.format( - arg.component_id - ) + dedent( + """ + component_id must be a string or dict, found {!r} + """ + ).format(arg.component_id) ) @@ -85,12 +86,12 @@ def validate_id_dict(arg): # cause unwanted collisions if not isinstance(k, _strings): raise exceptions.IncorrectTypeException( - """ - Wildcard ID keys must be non-empty strings, - found {!r} in id {!r} - """.format( - k, arg_id - ) + dedent( + """ + Wildcard ID keys must be non-empty strings, + found {!r} in id {!r} + """ + ).format(k, arg_id) ) @@ -113,13 +114,13 @@ def validate_id_string(arg): def validate_multi_return(outputs_list, output_value, callback_id): if not isinstance(output_value, (list, tuple)): raise exceptions.InvalidCallbackReturnValue( - """ - The callback {} is a multi-output. - Expected the output type to be a list or tuple but got: - {}. - """.format( - callback_id, repr(output_value) - ) + dedent( + """ + The callback {} is a multi-output. + Expected the output type to be a list or tuple but got: + {}. + """ + ).format(callback_id, repr(output_value)) ) if len(output_value) != len(outputs_list): @@ -137,26 +138,26 @@ def validate_multi_return(outputs_list, output_value, callback_id): vi = output_value[i] if not isinstance(vi, (list, tuple)): raise exceptions.InvalidCallbackReturnValue( - """ - The callback {} output {} is a wildcard multi-output. - Expected the output type to be a list or tuple but got: - {}. - output spec: {} - """.format( - callback_id, i, repr(vi), repr(outi) - ) + dedent( + """ + The callback {} output {} is a wildcard multi-output. + Expected the output type to be a list or tuple but got: + {}. + output spec: {} + """ + ).format(callback_id, i, repr(vi), repr(outi)) ) if len(vi) != len(outi): raise exceptions.InvalidCallbackReturnValue( - """ - Invalid number of output values for {} item {}. - Expected {}, got {} - output spec: {} - output value: {} - """.format( - callback_id, i, len(vi), len(outi), repr(outi), repr(vi) - ) + dedent( + """ + Invalid number of output values for {} item {}. + Expected {}, got {} + output spec: {} + output value: {} + """ + ).format(callback_id, i, len(vi), len(outi), repr(outi), repr(vi)) ) @@ -170,34 +171,38 @@ def _raise_invalid(bad_val, outer_val, path, index=None, toplevel=False): ) outer_type = type(outer_val).__name__ if toplevel: - location = """ - The value in question is either the only value returned, - or is in the top level of the returned list, - """ + location = dedent( + """ + The value in question is either the only value returned, + or is in the top level of the returned list, + """ + ) else: index_string = "[*]" if index is None else "[{:d}]".format(index) - location = """ - The value in question is located at - {} {} {} - {}, - """.format( - index_string, outer_type, outer_id, path - ) + location = dedent( + """ + The value in question is located at + {} {} {} + {}, + """ + ).format(index_string, outer_type, outer_id, path) raise exceptions.InvalidCallbackReturnValue( - """ - The callback for `{output}` - returned a {object:s} having type `{type}` - which is not JSON serializable. + dedent( + """ + The callback for `{output}` + returned a {object:s} having type `{type}` + which is not JSON serializable. - {location} - and has string representation - `{bad_val}` + {location} + and has string representation + `{bad_val}` - In general, Dash properties can only be - dash components, strings, dictionaries, numbers, None, - or lists of those. - """.format( + In general, Dash properties can only be + dash components, strings, dictionaries, numbers, None, + or lists of those. + """ + ).format( output=repr(output), object="tree with one value" if not toplevel else "value", type=bad_type, diff --git a/dash/resources.py b/dash/resources.py index 5d0d8a1ac6..65f8e088a4 100644 --- a/dash/resources.py +++ b/dash/resources.py @@ -24,8 +24,12 @@ def _filter_resources(self, all_resources, dev_bundles=False): if "async" in s: if "dynamic" in s: raise exceptions.ResourceException( - "Can't have both 'dynamic' and 'async'. " - "{}".format(json.dumps(filtered_resource)) + """ + Can't have both 'dynamic' and 'async'. + {} + """.format( + json.dumps(filtered_resource) + ) ) # Async assigns a value dynamically to 'dynamic' @@ -70,9 +74,12 @@ def _filter_resources(self, all_resources, dev_bundles=False): continue else: raise exceptions.ResourceException( - "{} does not have a " - "relative_package_path, absolute_path, or an " - "external_url.".format(json.dumps(filtered_resource)) + """ + {} does not have a relative_package_path, absolute_path, + or an external_url. + """.format( + json.dumps(filtered_resource) + ) ) filtered_resources.append(filtered_resource) From cc04375fdcb3f5fdd4278c9ae064bb9e1b7255a7 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Sat, 16 Jan 2021 20:52:36 -0500 Subject: [PATCH 2/2] changelog for error message dedent also fix changelog for #1528 --- CHANGELOG.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b4132423a..d0c1681e37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ to close the error messages box. - [#1503](https://github.com/plotly/dash/pull/1506) Fix [#1466](https://github.com/plotly/dash/issues/1466): loosen `dash[testing]` requirements for easier integration in external projects. This PR also bumps many `dash[dev]` requirements. ### Fixed +- [#1530](https://github.com/plotly/dash/pull/1530) Dedent error messages more carefully. +- [#1527](https://github.com/plotly/dash/issues/1527)🐛 `get_asset_url` now pulls from an external source if `assets_external_path` is set. + - updated `_add_assets_resource` to build asset urls the same way as `get_asset_url`. + - updated doc string for `assets_external_path` Dash argument to be more clear that it will allways be joined with the `assets_url_path` argument when determining the url to an external asset. - [#1493](https://github.com/plotly/dash/pull/1493) Fix [#1143](https://github.com/plotly/dash/issues/1143), a bug where having a file with one of several common names (test.py, code.py, org.py, etc) that imports a dash component package would make `import dash` fail with a cryptic error message asking whether you have a file named "dash.py" ## [1.18.1] - 2020-12-09 @@ -695,9 +699,3 @@ app = dash.Dash(...) ## 0.17.3 - 2017-06-22 ✨ This is the initial open-source release of Dash. - -### Fixed -- [#1527](https://github.com/plotly/dash/issues/1527)🐛 `get_asset_url` now pulls from an external source if `assets_external_path` is set. -- updated `_add_assets_resource` to build asset urls the same way as `get_asset_url`. -- updated doc string for `assets_external_path` Dash argument to be more clear that it will allways be joined with -the `assets_url_path` argument when determining the url to an external asset. \ No newline at end of file