From 716b3d9384fdfbfc494ab6dc6398212ebc11b030 Mon Sep 17 00:00:00 2001 From: Steven Wu Date: Fri, 20 Dec 2024 16:03:07 -0500 Subject: [PATCH 1/2] fix: label prop typing (#1071) - Fixes #988 - Adds `NodeType`, which mirrors the React Node type --- plugins/ui/src/deephaven/ui/components/combo_box.py | 4 ++-- plugins/ui/src/deephaven/ui/components/date_field.py | 4 ++-- plugins/ui/src/deephaven/ui/components/date_picker.py | 4 ++-- .../ui/src/deephaven/ui/components/date_range_picker.py | 4 ++-- plugins/ui/src/deephaven/ui/components/picker.py | 4 ++-- plugins/ui/src/deephaven/ui/components/progress_bar.py | 7 +++---- plugins/ui/src/deephaven/ui/components/time_field.py | 4 ++-- plugins/ui/src/deephaven/ui/elements/Element.py | 6 +++++- plugins/ui/src/deephaven/ui/elements/__init__.py | 2 +- 9 files changed, 21 insertions(+), 18 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index e7ddadc60..b8d6514cd 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -27,7 +27,7 @@ from .section import SectionElement from .item import Item from .item_table_source import ItemTableSource -from ..elements import BaseElement, Element +from ..elements import BaseElement, Element, NodeType from .._internal.utils import create_props, unpack_item_table_source from ..types import Key, Undefined, UndefinedType from .basic import component_element @@ -67,7 +67,7 @@ def combo_box( is_required: bool | None = None, validation_behavior: ValidationBehavior = "aria", auto_focus: bool | None = None, - label: Element | None = None, + label: NodeType = None, description: Element | None = None, error_message: Element | None = None, name: str | None = None, diff --git a/plugins/ui/src/deephaven/ui/components/date_field.py b/plugins/ui/src/deephaven/ui/components/date_field.py index 14fecfd08..8dafbd244 100644 --- a/plugins/ui/src/deephaven/ui/components/date_field.py +++ b/plugins/ui/src/deephaven/ui/components/date_field.py @@ -20,7 +20,7 @@ Alignment, ) -from ..elements import Element +from ..elements import Element, NodeType from .._internal.utils import ( create_props, convert_date_props, @@ -93,7 +93,7 @@ def date_field( is_required: bool | None = None, validation_behavior: ValidationBehavior | None = None, auto_focus: bool | None = None, - label: Element | None = None, + label: NodeType = None, description: Element | None = None, error_message: Element | None = None, is_open: bool | None = None, diff --git a/plugins/ui/src/deephaven/ui/components/date_picker.py b/plugins/ui/src/deephaven/ui/components/date_picker.py index 305248db6..ab886b40a 100644 --- a/plugins/ui/src/deephaven/ui/components/date_picker.py +++ b/plugins/ui/src/deephaven/ui/components/date_picker.py @@ -22,7 +22,7 @@ ) from ..hooks import use_memo -from ..elements import Element +from ..elements import Element, NodeType from .._internal.utils import ( create_props, convert_date_props, @@ -98,7 +98,7 @@ def date_picker( is_required: bool | None = None, validation_behavior: ValidationBehavior | None = None, auto_focus: bool | None = None, - label: Element | None = None, + label: NodeType = None, description: Element | None = None, error_message: Element | None = None, is_open: bool | None = None, diff --git a/plugins/ui/src/deephaven/ui/components/date_range_picker.py b/plugins/ui/src/deephaven/ui/components/date_range_picker.py index 6f8a1f99b..45d4fb950 100644 --- a/plugins/ui/src/deephaven/ui/components/date_range_picker.py +++ b/plugins/ui/src/deephaven/ui/components/date_range_picker.py @@ -22,7 +22,7 @@ ) from ..hooks import use_memo -from ..elements import Element +from ..elements import Element, NodeType from .._internal.utils import ( create_props, convert_date_props, @@ -96,7 +96,7 @@ def date_range_picker( is_required: bool | None = None, validation_behavior: ValidationBehavior | None = None, auto_focus: bool | None = None, - label: Element | None = None, + label: NodeType = None, description: Element | None = None, error_message: Element | None = None, is_open: bool | None = None, diff --git a/plugins/ui/src/deephaven/ui/components/picker.py b/plugins/ui/src/deephaven/ui/components/picker.py index f24f1c96b..cc02c583d 100644 --- a/plugins/ui/src/deephaven/ui/components/picker.py +++ b/plugins/ui/src/deephaven/ui/components/picker.py @@ -6,7 +6,7 @@ from .basic import component_element from .section import SectionElement, Item from .item_table_source import ItemTableSource -from ..elements import BaseElement, Element +from ..elements import BaseElement, Element, NodeType from .._internal.utils import create_props, unpack_item_table_source from ..types import Key, Undefined, UndefinedType from .types import ( @@ -60,7 +60,7 @@ def picker( validation_behavior: ValidationBehavior | None = None, description: Element | None = None, error_message: Element | None = None, - label: Element | None = None, + label: NodeType = None, placeholder: str | None = None, is_loading: bool | None = None, label_position: LabelPosition = "top", diff --git a/plugins/ui/src/deephaven/ui/components/progress_bar.py b/plugins/ui/src/deephaven/ui/components/progress_bar.py index ee2a7ef28..9c2945165 100644 --- a/plugins/ui/src/deephaven/ui/components/progress_bar.py +++ b/plugins/ui/src/deephaven/ui/components/progress_bar.py @@ -14,9 +14,8 @@ Position, ProgressBarSize, ) - from .basic import component_element -from ..elements import Element +from ..elements import Element, NodeType ProgressBarElement = Element @@ -26,9 +25,9 @@ def progress_bar( static_color: StaticColor | None = None, label_position: LabelPosition = "top", show_value_label: bool | None = None, - label: Element | None = None, + label: NodeType = None, # format_options, # omitted because need to connect it to Deephaven formatting options as well - value_label: Element | None = None, + value_label: NodeType = None, value: float = 0, min_value: float = 0, max_value: float = 100, diff --git a/plugins/ui/src/deephaven/ui/components/time_field.py b/plugins/ui/src/deephaven/ui/components/time_field.py index 7e4002d8c..91d1129d5 100644 --- a/plugins/ui/src/deephaven/ui/components/time_field.py +++ b/plugins/ui/src/deephaven/ui/components/time_field.py @@ -20,7 +20,7 @@ Alignment, ) -from ..elements import Element +from ..elements import Element, NodeType from .._internal.utils import ( create_props, convert_time_props, @@ -86,7 +86,7 @@ def time_field( is_required: bool | None = None, validation_behavior: ValidationBehavior | None = None, auto_focus: bool | None = None, - label: Element | None = None, + label: NodeType = None, description: Element | None = None, error_message: Element | None = None, name: str | None = None, diff --git a/plugins/ui/src/deephaven/ui/elements/Element.py b/plugins/ui/src/deephaven/ui/elements/Element.py index fe6a168f9..d094ae7fc 100644 --- a/plugins/ui/src/deephaven/ui/elements/Element.py +++ b/plugins/ui/src/deephaven/ui/elements/Element.py @@ -1,7 +1,7 @@ from __future__ import annotations from abc import ABC, abstractmethod -from typing import Any, Dict +from typing import Any, Dict, List, Union from .._internal import RenderContext PropsType = Dict[str, Any] @@ -45,3 +45,7 @@ def render(self, context: RenderContext) -> PropsType: The props of this element. """ pass + + +# Some props don't support Undefined, so they need to add it themselves +NodeType = Union[None, bool, int, str, Element, List["NodeType"]] diff --git a/plugins/ui/src/deephaven/ui/elements/__init__.py b/plugins/ui/src/deephaven/ui/elements/__init__.py index 21143a2f9..8065f588d 100644 --- a/plugins/ui/src/deephaven/ui/elements/__init__.py +++ b/plugins/ui/src/deephaven/ui/elements/__init__.py @@ -1,4 +1,4 @@ -from .Element import Element, PropsType +from .Element import Element, PropsType, NodeType from .BaseElement import BaseElement from .DashboardElement import DashboardElement from .FunctionElement import FunctionElement From 6b261d607cbb3fc826da4e4069471fc57cce3cd2 Mon Sep 17 00:00:00 2001 From: Joe Date: Mon, 23 Dec 2024 12:55:29 -0600 Subject: [PATCH 2/2] fix: Allow autodoc functions to have no parameters (#1072) Fixes https://deephaven.atlassian.net/browse/DH-18247 Allows autodoc functions to have no parameters and adds better error checking for params to ensure that all expected parameters are there in a fine-tuned way. Tested on all existing docs as well as a local `use_render_queue` doc. --- .../deephaven/ui/components/action_menu.py | 2 +- .../ui/src/deephaven/ui/components/column.py | 2 +- sphinx_ext/deephaven_autodoc.py | 56 ++++++++++++++----- 3 files changed, 43 insertions(+), 17 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/action_menu.py b/plugins/ui/src/deephaven/ui/components/action_menu.py index 2eca69c3b..10bb485ca 100644 --- a/plugins/ui/src/deephaven/ui/components/action_menu.py +++ b/plugins/ui/src/deephaven/ui/components/action_menu.py @@ -91,7 +91,7 @@ def action_menu( ActionMenu combines an ActionButton with a Menu for simple "more actions" use cases. Args: - children: The contents of the collection. + *children: The contents of the collection. is_disabled: Whether the button is disabled. is_quiet: Whether the button should be displayed with a quiet style. auto_focus: Whether the element should receive focus on render. diff --git a/plugins/ui/src/deephaven/ui/components/column.py b/plugins/ui/src/deephaven/ui/components/column.py index 4dbeea67b..85d9a981a 100644 --- a/plugins/ui/src/deephaven/ui/components/column.py +++ b/plugins/ui/src/deephaven/ui/components/column.py @@ -13,7 +13,7 @@ def column( Each element will be placed below its prior sibling. Args: - children: Elements to render in the column. + *children: Elements to render in the column. width: The percent width of the column relative to other children of its parent. If not provided, the column will be sized automatically. key: A unique identifier used by React to render elements in a list. diff --git a/sphinx_ext/deephaven_autodoc.py b/sphinx_ext/deephaven_autodoc.py index 56e6e946b..d987a12c2 100644 --- a/sphinx_ext/deephaven_autodoc.py +++ b/sphinx_ext/deephaven_autodoc.py @@ -46,10 +46,13 @@ class SignatureData(TypedDict): AUTOFUNCTION_COMMENT_PREFIX = "AutofunctionCommentPrefix:" +# Some parameters don't need to be documented such as function parameter dividers +UNDOCUMENTED_PARAMS = {"*", "/"} + def extract_parameter_defaults( node: sphinx.addnodes.desc_parameterlist, -) -> ParamDefaults: +) -> tuple[ParamDefaults, set[str]]: """ Extract the default values for the parameters from the parameter list @@ -57,20 +60,24 @@ def extract_parameter_defaults( node: The node to extract the defaults from Returns: - The parameter defaults + The parameter defaults and the expected parameters for comparison """ defaults = {} + expected_params = set() for child in node.children: params = child.astext().split("=") if len(params) == 2: defaults[params[0]] = params[1] - # otherwise, no default value - do not add it because None is not the same as no default - return defaults + # otherwise, no default value - do not add it to defaults because None is not the same as no default + # but add it to expected_params so that we can check that all parameters are documented + expected_params.add(params[0]) + + return defaults, expected_params def extract_signature_data( node: sphinx.addnodes.desc_signature, -) -> tuple[FunctionMetadata, ParamDefaults]: +) -> tuple[FunctionMetadata, ParamDefaults, set[str]]: """ Extract the signature data from the signature node The default values for the parameters are extracted from the signature @@ -79,18 +86,19 @@ def extract_signature_data( node: The node to extract the signature data from Returns: - The function metadata and the parameter defaults + The function metadata, the parameter defaults, and the expected parameters """ result = {} param_defaults = {} + expected_params = set() for child in node.children: if isinstance(child, sphinx.addnodes.desc_addname): result["module_name"] = child.astext() elif isinstance(child, sphinx.addnodes.desc_name): result["name"] = child.astext() elif isinstance(child, sphinx.addnodes.desc_parameterlist): - param_defaults = extract_parameter_defaults(child) - return FunctionMetadata(**result), param_defaults + param_defaults, expected_params = extract_parameter_defaults(child) + return FunctionMetadata(**result), param_defaults, expected_params def extract_list_item(node: docutils.nodes.list_item) -> ParamData: @@ -108,7 +116,7 @@ def extract_list_item(node: docutils.nodes.list_item) -> ParamData: match = re.match(r"(.+?) \((.*?)\) -- (.+)", field, re.DOTALL) if match is None: raise ValueError( - f"Could not match {field} to extract param data. " + f"Could not match '{field}' to extract param data. " f"Verify this parameter is documented correctly within 'Args:' with type and description." ) matched = match.groups() @@ -245,6 +253,18 @@ def attach_parameter_defaults(params: Params, param_defaults: ParamDefaults) -> param["default"] = param_defaults[name] +def missing_parameters(params: Params, expected_params: set[str]) -> set[str]: + """ + Get the parameters that are missing from the documentation + + Args: + params: The parameters that are documented + expected_params: The parameters that are expected + """ + param_names = set(param["name"] for param in params) + return expected_params - UNDOCUMENTED_PARAMS - param_names + + def extract_desc_data(node: sphinx.addnodes.desc) -> SignatureData: """ Extract the content of the description. @@ -258,21 +278,27 @@ def extract_desc_data(node: sphinx.addnodes.desc) -> SignatureData: """ result = {} param_defaults = {} + params_expected = set() for child_node in node.children: if isinstance(child_node, sphinx.addnodes.desc_signature): - signature_results, param_defaults = extract_signature_data(child_node) + signature_results, param_defaults, params_expected = extract_signature_data( + child_node + ) result.update(signature_results) elif isinstance(child_node, sphinx.addnodes.desc_content): result.update(extract_content_data(child_node)) # map all to lowercase for consistency function = f"{result['module_name']}{result['name']}" - try: - result["parameters"] = result.pop("Parameters") - except KeyError: + + result["parameters"] = result.pop("Parameters") if "Parameters" in result else [] + missing_params = missing_parameters(result["parameters"], params_expected) + + if missing_params: raise ValueError( - "Parameters missing from description. " - f"Verify the function description for {function} is formatted correctly." + f"Missing documentation for {function} parameters {missing_params}. " + "Verify that parameter names have leading asterisks in the description." ) + try: result["return_description"] = result.pop("Returns") except KeyError: