Skip to content
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

Limit generated components to 250 explicit args by default #2029

Merged
merged 5 commits into from
Apr 28, 2022
Merged
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ npm-debug*
dash_generator_test_component_standard/
dash_generator_test_component_nested/
dash_test_components/
dash_generator_test_component_typescript/
inst/
man/
R/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"scripts": {
"build:js": "webpack --mode production",
"setup": "python setup.py sdist",
"build:py_and_r": "dash-generate-components ./src/components dash_generator_test_component_standard && cp base/** dash_generator_test_component_standard/ && dash-generate-components ./src/components dash_generator_test_component_standard --r-prefix 'dgtc_standard'",
"build:py_and_r": "dash-generate-components ./src/components dash_generator_test_component_standard && cp base/** dash_generator_test_component_standard/ && dash-generate-components ./src/components dash_generator_test_component_standard --r-prefix 'dgtc_standard' --max-props 2",
"build": "run-s build:js build:py_and_r setup"
},
"author": "Chris Parmer <[email protected]>",
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).

### Fixed

- [#2029](https://github.com/plotly/dash/pull/2029) Restrict the number of props listed explicitly in generated component constructors - default is 250. This prevents exceeding the Python 3.6 limit of 255 arguments. The omitted props are still in the docstring and can still be provided the same as before, they just won't appear in the signature so autocompletion may be affected.

- [#1968](https://github.com/plotly/dash/pull/1968) Fix bug [#1877](https://github.com/plotly/dash/issues/1877), code which uses `merge_duplicate_headers` and `style_header_conditional` to highlight columns, it incorrectly highlights header cells.

- [#2015](https://github.com/plotly/dash/pull/2015) Fix bug [#1854](https://github.com/plotly/dash/issues/1854) in which the combination of row_selectable="single or multi" and filter_action="native" caused the JS error.
Expand Down
52 changes: 36 additions & 16 deletions dash/development/_py_components_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@
from .base_component import Component


# pylint: disable=unused-argument
# pylint: disable=unused-argument,too-many-locals
def generate_class_string(
typename, props, description, namespace, prop_reorder_exceptions=None
typename,
props,
description,
namespace,
prop_reorder_exceptions=None,
max_props=None,
):
"""Dynamically generate class strings to have nicely formatted docstrings,
keyword arguments, and repr.
Expand Down Expand Up @@ -56,7 +61,7 @@ def __init__(self, {default_argtext}):
{list_of_valid_wildcard_attr_prefixes}
_explicit_args = kwargs.pop('_explicit_args')
_locals = locals()
_locals.update(kwargs) # For wildcard attrs
_locals.update(kwargs) # For wildcard attrs and excess named props
args = {{k: _locals[k] for k in _explicit_args if k != 'children'}}
for k in {required_props}:
if k not in args:
Expand Down Expand Up @@ -91,18 +96,28 @@ def __init__(self, {default_argtext}):
else:
default_argtext = ""
argtext = "**args"
default_argtext += ", ".join(
[
(
f"{p:s}=Component.REQUIRED"
if props[p]["required"]
else f"{p:s}=Component.UNDEFINED"
default_arglist = [
(
f"{p:s}=Component.REQUIRED"
if props[p]["required"]
else f"{p:s}=Component.UNDEFINED"
)
for p in prop_keys
if not p.endswith("-*") and p not in python_keywords and p != "setProps"
]

if max_props:
final_max_props = max_props - (1 if "children" in props else 0)
if len(default_arglist) > final_max_props:
default_arglist = default_arglist[:final_max_props]
docstring += (
"\n\n"
"Note: due to the large number of props for this component,\n"
"not all of them appear in the constructor signature, but\n"
"they may still be used as keyword arguments."
)
for p in prop_keys
if not p.endswith("-*") and p not in python_keywords and p != "setProps"
]
+ ["**kwargs"]
)

default_argtext += ", ".join(default_arglist + ["**kwargs"])
required_args = required_props(filtered_props)
return c.format(
typename=typename,
Expand All @@ -118,7 +133,12 @@ def __init__(self, {default_argtext}):


def generate_class_file(
typename, props, description, namespace, prop_reorder_exceptions=None
typename,
props,
description,
namespace,
prop_reorder_exceptions=None,
max_props=None,
):
"""Generate a Python class file (.py) given a class string.
Parameters
Expand All @@ -138,7 +158,7 @@ def generate_class_file(
)

class_string = generate_class_string(
typename, props, description, namespace, prop_reorder_exceptions
typename, props, description, namespace, prop_reorder_exceptions, max_props
)
file_name = f"{typename:s}.py"

Expand Down
2 changes: 1 addition & 1 deletion dash/development/base_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def __init__(self, **kwargs):
+ " detected a Component for a prop other than `children`\n"
+ f"Prop {k} has value {v!r}\n\n"
+ "Did you forget to wrap multiple `children` in an array?\n"
+ "For example, it must be html.Div([\"a\", \"b\", \"c\"]) not html.Div(\"a\", \"b\", \"c\")\n"
+ 'For example, it must be html.Div(["a", "b", "c"]) not html.Div("a", "b", "c")\n'
)

if k == "id":
Expand Down
34 changes: 24 additions & 10 deletions dash/development/component_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class _CombinedFormatter(
pass


# pylint: disable=too-many-locals, too-many-arguments, too-many-branches
# pylint: disable=too-many-locals, too-many-arguments, too-many-branches, too-many-statements
def generate_components(
components_source,
project_shortname,
Expand All @@ -48,6 +48,7 @@ def generate_components(
jlprefix=None,
metadata=None,
keep_prop_order=None,
max_props=None,
):

project_shortname = project_shortname.replace("-", "_").rstrip("/\\")
Expand Down Expand Up @@ -97,7 +98,17 @@ def generate_components(

metadata = safe_json_loads(out.decode("utf-8"))

generator_methods = [generate_class_file]
py_generator_kwargs = {}
if keep_prop_order is not None:
keep_prop_order = [
component.strip(" ") for component in keep_prop_order.split(",")
]
py_generator_kwargs["prop_reorder_exceptions"] = keep_prop_order

if max_props:
py_generator_kwargs["max_props"] = max_props

generator_methods = [functools.partial(generate_class_file, **py_generator_kwargs)]

if rprefix is not None or jlprefix is not None:
with open("package.json", "r") as f:
Expand All @@ -122,14 +133,6 @@ def generate_components(
functools.partial(generate_struct_file, prefix=jlprefix)
)

if keep_prop_order is not None:
keep_prop_order = [
component.strip(" ") for component in keep_prop_order.split(",")
]
generator_methods[0] = functools.partial(
generate_class_file, prop_reorder_exceptions=keep_prop_order
)

components = generate_classes_files(project_shortname, metadata, *generator_methods)

with open(os.path.join(project_shortname, "metadata.json"), "w") as f:
Expand Down Expand Up @@ -221,6 +224,16 @@ def component_build_arg_parser():
"props. Pass the 'ALL' keyword to have every component retain "
"its original prop order.",
)
parser.add_argument(
"--max-props",
type=int,
default=250,
help="Specify the max number of props to list in the component signature. "
"More props will still be shown in the docstring, and will still work when "
"provided as kwargs to the component. Python <3.7 only supports 255 args, "
"but you may also want to reduce further for improved readability at the "
"expense of auto-completion for the later props. Use 0 to include all props.",
)
return parser


Expand All @@ -237,6 +250,7 @@ def cli():
rsuggests=args.r_suggests,
jlprefix=args.jl_prefix,
keep_prop_order=args.keep_prop_order,
max_props=args.max_props,
)


Expand Down
22 changes: 22 additions & 0 deletions tests/integration/test_generation.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pytest

from dash import Dash, Input, Output
from dash.exceptions import PreventUpdate

Expand All @@ -19,6 +21,9 @@ def test_gene001_simple_callback(dash_duo):

app.layout = Div(
[
# Note: `value` is not part of the explicit function signature
# for MyStandardComponent, due to --max-props 2 in its build command
# but this verifies that it still works.
MyStandardComponent(id="standard", value="Standard"),
MyNestedComponent(id="nested", value="Nested"),
TypeScriptComponent(id="typescript", required_string="TypeScript"),
Expand Down Expand Up @@ -72,3 +77,20 @@ def update_container(n_clicks):
)

dash_duo.percy_snapshot(name="gene002-arbitrary-resource")


def test_gene003_max_props():
limited_props_doc = "large number of props for this component"
# dash_generator_test_component_standard has max_props set to 2, so
# MyStandardComponent gets the restricted signature and note about it
# in its docstring.
# dash_generator_test_component_nested and MyNestedComponent do not.
assert limited_props_doc in MyStandardComponent.__doc__
assert limited_props_doc not in MyNestedComponent.__doc__

# Verify that it still works to check for invalid props in both cases
with pytest.raises(TypeError):
MyStandardComponent(valuex="nope")

with pytest.raises(TypeError):
MyNestedComponent(valuey="nor this")
2 changes: 1 addition & 1 deletion tests/unit/development/metadata_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def __init__(self, children=None, optionalArray=Component.UNDEFINED, optionalBoo
self.available_wildcard_properties = ['data-', 'aria-']
_explicit_args = kwargs.pop('_explicit_args')
_locals = locals()
_locals.update(kwargs) # For wildcard attrs
_locals.update(kwargs) # For wildcard attrs and excess named props
args = {k: _locals[k] for k in _explicit_args if k != 'children'}
for k in []:
if k not in args:
Expand Down