Merge pull request #2029 from plotly/limit-component-args

Limit generated components to 250 explicit args by default

Author: alexcjohnson

.gitignore

@@ -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/

@plotly/dash-generator-test-component-standard/package.json

@@ -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]>",

CHANGELOG.md

@@ -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.

dash/development/_py_components_generation.py

@@ -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.
@@ -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:
@@ -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,
@@ -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
@@ -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"
 

dash/development/base_component.py

@@ -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":

dash/development/component_generator.py

@@ -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,
@@ -48,6 +48,7 @@ def generate_components(
     jlprefix=None,
     metadata=None,
     keep_prop_order=None,
+    max_props=None,
 ):
 
     project_shortname = project_shortname.replace("-", "_").rstrip("/\\")
@@ -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:
@@ -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:
@@ -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
 
 
@@ -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,
     )
 
 

tests/integration/test_generation.py

@@ -1,3 +1,5 @@
+import pytest
+
 from dash import Dash, Input, Output
 from dash.exceptions import PreventUpdate
 
@@ -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"),
@@ -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")

tests/unit/development/metadata_test.py

@@ -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: