Merge branch 'main' into tidy/scriv

Signed-off-by: Antony Milne <[email protected]>

vizro-core/changelog.d/20231024_101744_antony.milne_captured_callable.md

@@ -0,0 +1,41 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+
+### Fixed
+
+- `CapturedCallable` now handles variadic keywords arguments (`**kwargs`) correctly ([#121](https://github.com/mckinsey/vizro/pull/121))
+
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

vizro-core/changelog.d/20231025_125539_huong_li_nguyen_add_disclaimer_for_custom_created.md

@@ -0,0 +1,42 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Removed
+
+- A bullet item for the Removed category.
+
+-->
+<!--
+### Added
+
+- A bullet item for the Added category.
+
+-->
+<!--
+### Changed
+
+- A bullet item for the Changed category.
+
+-->
+<!--
+### Deprecated
+
+- A bullet item for the Deprecated category.
+
+-->
+<!--
+### Fixed
+
+- A bullet item for the Fixed category.
+
+-->
+<!--
+### Security
+
+- A bullet item for the Security category.
+
+-->

vizro-core/docs/pages/user_guides/custom_charts.md

@@ -153,3 +153,10 @@ The below examples shows a more involved use-case. We create and style a waterfa
         [![Graph3]][Graph3]
 
     [Graph3]: ../../assets/user_guides/custom_charts/custom_chart_waterfall.png
+
+
+???+ warning
+
+    Please note that users of this package are responsible for the content of any custom-created component,
+    function or integration they write - especially with regard to leaking any sensitive information or exposing to
+    any security threat during implementation.

vizro-core/docs/pages/user_guides/custom_components.md

@@ -7,11 +7,6 @@ In general, you can create a custom component based on any dash-compatible compo
 [dash-bootstrap-components](https://dash-bootstrap-components.opensource.faculty.ai/), [dash-html-components](https://github.com/plotly/dash/tree/dev/components/dash-html-components), etc.).
 
 
-!!!warning
-    When creating your own custom components, you are responsible for the security of your component (e.g. prevent setting HTML from code which might expose users to cross-site scripting). Vizro cannot guarantee
-    the security of custom created components, so make sure you keep this in mind when publicly deploying your dashboard.
-
-
 Vizro's public API is deliberately kept small in order to facilitate quick and easy configuration of a dashboard. However,
 at the same time, Vizro is easily extensible, so that you can tweak any component to your liking or even create entirely new ones.
 
@@ -354,3 +349,11 @@ vm.Page.add_type("components", Jumbotron)
         [![CustomComponent2]][CustomComponent2]
 
     [CustomComponent2]: ../../assets/user_guides/custom_components/customcomponent_2.png
+
+
+
+???+ warning
+
+    Please note that users of this package are responsible for the content of any custom-created component,
+    function or integration they write - especially with regard to leaking any sensitive information or exposing to
+    any security threat during implementation.

vizro-core/docs/pages/user_guides/integration.md

@@ -45,3 +45,11 @@ register the datasets with [`kedro_datasets.pandas`](https://docs.kedro.org/en/s
         for dataset_name, dataset in kedro_integration.datasets_from_catalog(catalog).items():
             data_manager[dataset_name] = dataset
         ```
+
+
+
+???+ warning
+
+    Please note that users of this package are responsible for the content of any custom-created component,
+    function or integration they write - especially with regard to leaking any sensitive information or exposing to
+    any security threat during implementation.

vizro-core/src/vizro/models/types.py

@@ -4,7 +4,6 @@
 
 import functools
 import inspect
-from copy import deepcopy
 from typing import Any, Dict, List, Literal, Protocol, Union, runtime_checkable
 
 from pydantic import Field, StrictBool
@@ -40,37 +39,72 @@ def __init__(self, function, /, *args, **kwargs):
         """Creates a new CapturedCallable object that will be able to re-run `function`.
 
         Partially binds *args and **kwargs to the function call.
+
+        Raises:
+            ValueError if `function` contains positional-only or variadic positional parameters (*args).
         """
+        # It is difficult to get positional-only and variadic positional arguments working at the same time as
+        # variadic keyword arguments. Ideally we would do the __call__ as
+        # self.__function(*bound_arguments.args, **bound_arguments.kwargs) as in the
+        # Python documentation. This would handle positional-only and variadic positional arguments better but makes
+        # it more difficult to handle variadic keyword arguments due to https://bugs.python.org/issue41745.
+        # Hence we abandon bound_arguments.args and bound_arguments.kwargs in favor of just using
+        # self.__function(**bound_arguments.arguments).
+        parameters = inspect.signature(function).parameters
+        invalid_params = {
+            param.name
+            for param in parameters.values()
+            if param.kind in [inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.VAR_POSITIONAL]
+        }
+
+        if invalid_params:
+            raise ValueError(
+                f"Invalid parameter {', '.join(invalid_params)}. CapturedCallable does not accept functions with "
+                f"positional-only or variadic positional parameters (*args)."
+            )
+
         self.__function = function
-        self.__bound_arguments = inspect.signature(function).bind_partial(*args, **kwargs)
+        self.__arguments = inspect.signature(function).bind_partial(*args, **kwargs).arguments
+
+        # A function can only ever have one variadic keyword parameter. {""} is just here so that var_keyword_param
+        # is always unpacking a one element set.
+        (var_keyword_param,) = {
+            param.name for param in parameters.values() if param.kind == inspect.Parameter.VAR_KEYWORD
+        } or {""}
+
+        # Since we do __call__ as self.__function(**bound_arguments.arguments), we need to restructure the arguments
+        # a bit to put the kwargs in the right place.
+        # For a function with parameter **kwargs this converts self.__arguments = {"kwargs": {"a": 1}} into
+        # self.__arguments = {"a": 1}.
+        if var_keyword_param in self.__arguments:
+            self.__arguments.update(self.__arguments[var_keyword_param])
+            del self.__arguments[var_keyword_param]
 
     def __call__(self, **kwargs):
         """Run the `function` with the initial arguments overridden by **kwargs.
 
         Note *args are not possible here, but you can still override positional arguments using argument name.
         """
-        if not kwargs:
-            return self.__function(*self.__bound_arguments.args, **self.__bound_arguments.kwargs)
-
-        bound_arguments = deepcopy(self.__bound_arguments)
-        bound_arguments.arguments.update(kwargs)
-        # This looks like it should be self.__function(*bound_arguments.args, **bound_arguments.kwargs) as in the
-        # Python documentation, but that leads to problems due to https://bugs.python.org/issue41745.
-        return self.__function(**bound_arguments.arguments)
+        return self.__function(**{**self.__arguments, **kwargs})
 
     def __getitem__(self, arg_name: str):
         """Gets the value of a bound argument."""
-        return self.__bound_arguments.arguments[arg_name]
+        return self.__arguments[arg_name]
 
     def __delitem__(self, arg_name: str):
         """Deletes a bound argument."""
-        del self.__bound_arguments.arguments[arg_name]
+        del self.__arguments[arg_name]
 
     @property
     def _arguments(self):
         # TODO: This is used twice: in _get_parametrized_config and in vm.Action and should be removed when those
         # references are removed.
-        return self.__bound_arguments.arguments
+        return self.__arguments
+
+    # TODO-actions: Find a way how to compare CapturedCallable and function
+    @property
+    def _function(self):
+        return self.__function
 
     @classmethod
     def __get_validators__(cls):
@@ -137,11 +171,6 @@ def _parse_json(
         else:
             raise ValueError(f"_target_={function_name} must be wrapped in the @capture decorator.")
 
-    # TODO-actions: Find a way how to compare CapturedCallable and function
-    @property
-    def _function(self):
-        return self.__function
-
 
 class capture:
     """Captures a function call to create a [`CapturedCallable`][vizro.models.types.CapturedCallable].
@@ -175,6 +204,8 @@ def __call__(self, func, /):
             # The more difficult case, where we need to still have a valid plotly figure that renders in a notebook.
             # Hence we attach the CapturedCallable as a property instead of returning it directly.
             # TODO: move point of checking that data_frame argument exists earlier on.
+            # TODO: also would be nice to raise errors in CapturedCallable.__init__ at point of function definition
+            #  rather than point of calling if possible.
             @functools.wraps(func)
             def wrapped(*args, **kwargs) -> _DashboardReadyFigure:
                 if "data_frame" not in inspect.signature(func).parameters:
@@ -278,7 +309,8 @@ class OptionsDictType(TypedDict):
 NavigationPagesType = Annotated[
     Union[List[str], Dict[str, List[str]]],
     Field(
-        None, description="List of Page IDs or dict mapping of Page IDs and titles (for hierarchical sub-navigation)"
+        None,
+        description="List of Page IDs or dict mapping of Page IDs and titles (for hierarchical sub-navigation)",
     ),
 ]
 """Permissible value types for page attribute. Values are displayed as default."""