-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
1193 multipage validation #1201
Changes from 4 commits
7f164fd
30ff915
ce6a82c
e6547a5
241c047
00e6e73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -331,7 +331,8 @@ def __init__( | |
self.routes = [] | ||
|
||
self._layout = None | ||
self._cached_layout = None | ||
self._layout_is_function = False | ||
self.validation_layout = None | ||
|
||
self._setup_dev_tools() | ||
self._hot_reload = AttributeDict( | ||
|
@@ -421,18 +422,43 @@ def layout(self): | |
return self._layout | ||
|
||
def _layout_value(self): | ||
if isinstance(self._layout, patch_collections_abc("Callable")): | ||
self._cached_layout = self._layout() | ||
else: | ||
self._cached_layout = self._layout | ||
return self._cached_layout | ||
return self._layout() if self._layout_is_function else self._layout | ||
|
||
@layout.setter | ||
def layout(self, value): | ||
_validate.validate_layout_type(value) | ||
self._cached_layout = None | ||
self._layout_is_function = isinstance(value, patch_collections_abc("Callable")) | ||
self._layout = value | ||
|
||
# for using flask.has_request_context() to deliver a full layout for | ||
# validation inside a layout function - track if a user might be doing this. | ||
if self._layout_is_function and not self.validation_layout: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this is something we should turn off if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Those mostly wouldn't have IDs, so wouldn't be sent to the front end anyway, but with debug off we don't need it at all. In fact I guess we should be able to short-circuit all the front-end validation in that case too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added clauses to skip this when |
||
|
||
def simple_clone(c, children=None): | ||
cls = type(c) | ||
# in Py3 we can use the __init__ signature to reduce to just | ||
# required args and id; in Py2 this doesn't work so we just | ||
# empty out children. | ||
sig = getattr(cls.__init__, "__signature__", None) | ||
props = { | ||
p: getattr(c, p) | ||
for p in c._prop_names # pylint: disable=protected-access | ||
if hasattr(c, p) | ||
and ( | ||
p == "id" or not sig or sig.parameters[p].default == c.REQUIRED | ||
) | ||
} | ||
if props.get("children", children): | ||
props["children"] = children or [] | ||
return cls(**props) | ||
|
||
layout_value = self._layout_value() | ||
_validate.validate_layout(value, layout_value) | ||
self.validation_layout = simple_clone( | ||
# pylint: disable=protected-access | ||
layout_value, [simple_clone(c) for c in layout_value._traverse_ids()] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this validation layout as simple as possible, to minimize its overhead on the page:
This step is only run once when you set the layout, and even then only when you set it to a function AND you haven't already set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're just using this to validate IDs w.r.t. callbacks, why do we need the required values? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also validate props against the component type, so we can't just create dummy components, they have to be the same type as the real components. And unless I were to recreate the JSON serialization for components, the easiest way to do that is to actually instantiate the same component class. But doing so requires that we include required props, because the generated components enforce the required props during construction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ohhhhhh by "props against the component type" do you mean during callback validation with messages like e.g. "You registered a callback to update a Div's childrenz property. However, Div does not have a "children" property."? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
exactly. |
||
) | ||
|
||
@property | ||
def index_string(self): | ||
return self._index_string | ||
|
@@ -468,6 +494,9 @@ def _config(self): | |
"interval": int(self._dev_tools.hot_reload_interval * 1000), | ||
"max_retry": self._dev_tools.hot_reload_max_retry, | ||
} | ||
if self.validation_layout: | ||
config["validation_layout"] = self.validation_layout | ||
|
||
return config | ||
|
||
def serve_reload_hash(self): | ||
|
@@ -602,7 +631,7 @@ def _generate_scripts_html(self): | |
|
||
def _generate_config_html(self): | ||
return '<script id="_dash-config" type="application/json">{}</script>'.format( | ||
json.dumps(self._config()) | ||
json.dumps(self._config(), cls=plotly.utils.PlotlyJSONEncoder) | ||
) | ||
|
||
def _generate_renderer(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this because
validation_layout
is pruned down, so faster to crawl?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just saying if we have a separate validation layout, that's the one we need to check the callbacks against. The fact that it's pruned should make
computePaths
(next line) faster, but the main reason for pruning was to reduce the size of_dash-config
on the index page - I was imagining it could become enormous, if you have multiple tables and graphs on different pages all concatenated together.