From 8830cabee6c166694fc40a7ec0606e64165d66fb Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Sun, 28 Jul 2024 11:02:44 -0500 Subject: [PATCH 1/6] sync remote --- demo-project/.version | 0 src/components/export-modal/export-modal.js | 0 src/components/flowchart/flowchart.js | 0 src/components/flowchart/styles/_layers.scss | 0 src/components/global-toolbar/global-toolbar.js | 0 src/components/ui/button/button.js | 0 src/components/ui/dropdown/dropdown.js | 0 src/components/update-reminder/update-reminder-content.js | 0 src/config.js | 0 src/store/initial-state.js | 0 tools/test-lib/react-app/app.js | 0 11 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 demo-project/.version mode change 100644 => 100755 src/components/export-modal/export-modal.js mode change 100644 => 100755 src/components/flowchart/flowchart.js mode change 100644 => 100755 src/components/flowchart/styles/_layers.scss mode change 100644 => 100755 src/components/global-toolbar/global-toolbar.js mode change 100644 => 100755 src/components/ui/button/button.js mode change 100644 => 100755 src/components/ui/dropdown/dropdown.js mode change 100644 => 100755 src/components/update-reminder/update-reminder-content.js mode change 100644 => 100755 src/config.js mode change 100644 => 100755 src/store/initial-state.js mode change 100644 => 100755 tools/test-lib/react-app/app.js diff --git a/demo-project/.version b/demo-project/.version old mode 100644 new mode 100755 diff --git a/src/components/export-modal/export-modal.js b/src/components/export-modal/export-modal.js old mode 100644 new mode 100755 diff --git a/src/components/flowchart/flowchart.js b/src/components/flowchart/flowchart.js old mode 100644 new mode 100755 diff --git a/src/components/flowchart/styles/_layers.scss b/src/components/flowchart/styles/_layers.scss old mode 100644 new mode 100755 diff --git a/src/components/global-toolbar/global-toolbar.js b/src/components/global-toolbar/global-toolbar.js old mode 100644 new mode 100755 diff --git a/src/components/ui/button/button.js b/src/components/ui/button/button.js old mode 100644 new mode 100755 diff --git a/src/components/ui/dropdown/dropdown.js b/src/components/ui/dropdown/dropdown.js old mode 100644 new mode 100755 diff --git a/src/components/update-reminder/update-reminder-content.js b/src/components/update-reminder/update-reminder-content.js old mode 100644 new mode 100755 diff --git a/src/config.js b/src/config.js old mode 100644 new mode 100755 diff --git a/src/store/initial-state.js b/src/store/initial-state.js old mode 100644 new mode 100755 diff --git a/tools/test-lib/react-app/app.js b/tools/test-lib/react-app/app.js old mode 100644 new mode 100755 From fa44441d128cc5c9d36fd663994f2e2b0f54736f Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Tue, 1 Oct 2024 20:51:15 -0500 Subject: [PATCH 2/6] custom resolver fix initial draft --- demo-project/.version | 0 package/kedro_viz/api/apps.py | 3 +- .../data_access/repositories/catalog.py | 2 +- package/kedro_viz/models/flowchart.py | 12 +++++- package/kedro_viz/models/utils.py | 22 ++++++++++ package/tests/test_models/test_flowchart.py | 43 +++++++++++++++++++ package/tests/test_models/test_utils.py | 41 +++++++++++++++++- src/components/export-modal/export-modal.js | 0 src/components/flowchart/flowchart.js | 0 src/components/flowchart/styles/_layers.scss | 0 .../global-toolbar/global-toolbar.js | 0 src/components/ui/button/button.js | 0 src/components/ui/dropdown/dropdown.js | 0 .../update-reminder-content.js | 0 src/config.js | 0 src/store/initial-state.js | 0 tools/test-lib/react-app/app.js | 0 17 files changed, 118 insertions(+), 5 deletions(-) mode change 100755 => 100644 demo-project/.version mode change 100755 => 100644 src/components/export-modal/export-modal.js mode change 100755 => 100644 src/components/flowchart/flowchart.js mode change 100755 => 100644 src/components/flowchart/styles/_layers.scss mode change 100755 => 100644 src/components/global-toolbar/global-toolbar.js mode change 100755 => 100644 src/components/ui/button/button.js mode change 100755 => 100644 src/components/ui/dropdown/dropdown.js mode change 100755 => 100644 src/components/update-reminder/update-reminder-content.js mode change 100755 => 100644 src/config.js mode change 100755 => 100644 src/store/initial-state.js mode change 100755 => 100644 tools/test-lib/react-app/app.js diff --git a/demo-project/.version b/demo-project/.version old mode 100755 new mode 100644 diff --git a/package/kedro_viz/api/apps.py b/package/kedro_viz/api/apps.py index aef4d44715..d5b5c535ca 100644 --- a/package/kedro_viz/api/apps.py +++ b/package/kedro_viz/api/apps.py @@ -1,6 +1,7 @@ """`kedro_viz.api.app` defines the FastAPI app to serve Kedro data in a RESTful API. This data could either come from a real Kedro project or a file. """ + import json import os import time @@ -42,7 +43,7 @@ def _create_base_api_app() -> FastAPI: @app.middleware("http") async def set_secure_headers(request, call_next): response = await call_next(request) - secure_headers.framework.fastapi(response) # pylint: disable=no-member + secure_headers.framework.fastapi(response) return response return app diff --git a/package/kedro_viz/data_access/repositories/catalog.py b/package/kedro_viz/data_access/repositories/catalog.py index a7d568ca8a..d9367cbeb1 100644 --- a/package/kedro_viz/data_access/repositories/catalog.py +++ b/package/kedro_viz/data_access/repositories/catalog.py @@ -131,7 +131,7 @@ def get_dataset(self, dataset_name: str) -> Optional["AbstractDataset"]: else: # pragma: no cover dataset_obj = self._catalog._get_dataset(dataset_name) except DatasetNotFoundError: - dataset_obj = MemoryDataset() # pylint: disable=abstract-class-instantiated + dataset_obj = MemoryDataset() # type: ignore # pylint: disable=abstract-class-instantiated return dataset_obj diff --git a/package/kedro_viz/models/flowchart.py b/package/kedro_viz/models/flowchart.py index 9e6cb087d4..65a208a6c8 100644 --- a/package/kedro_viz/models/flowchart.py +++ b/package/kedro_viz/models/flowchart.py @@ -19,7 +19,7 @@ model_validator, ) -from kedro_viz.models.utils import get_dataset_type +from kedro_viz.models.utils import get_dataset_type, serialize_dict from kedro_viz.utils import TRANSCODING_SEPARATOR, _strip_transcoding try: @@ -861,7 +861,15 @@ def parameter_value(self) -> Any: return None try: - return self.kedro_obj.load() + actual_parameter_value = self.kedro_obj.load() + if self.is_all_parameters() and isinstance(actual_parameter_value, dict): + serialized_parameter_value = serialize_dict(actual_parameter_value) + return serialized_parameter_value + if isinstance(actual_parameter_value, (int, float)): + # handles a single parameter value which can be serialized + return actual_parameter_value + # handles any complex type that can't be serialized + return str(actual_parameter_value) except (AttributeError, DatasetError): # This except clause triggers if the user passes a parameter that is not # defined in the catalog (DatasetError) it also catches any case where diff --git a/package/kedro_viz/models/utils.py b/package/kedro_viz/models/utils.py index 1749946a3f..c84c32c6a2 100644 --- a/package/kedro_viz/models/utils.py +++ b/package/kedro_viz/models/utils.py @@ -1,4 +1,6 @@ """`kedro_viz.models.utils` contains utility functions used in the `kedro_viz.models` package""" + +import json import logging from typing import TYPE_CHECKING @@ -43,3 +45,23 @@ def get_dataset_type(dataset: "AbstractDataset") -> str: abbreviated_module_name = ".".join(dataset.__class__.__module__.split(".")[-2:]) class_name = f"{dataset.__class__.__qualname__}" return f"{abbreviated_module_name}.{class_name}" + + +def serialize_dict(original_dict: dict) -> dict: + """Serialize a dictionary by converting its values to strings + if the value is non serializable.""" + serialized_dict = {} + + for key, value in original_dict.items(): + if isinstance(value, dict): + # Recursively process the nested dictionary + serialized_dict[key] = serialize_dict(value) + else: + try: + # Check if the value is serializable + json.dumps(value) + serialized_dict[key] = value + except (TypeError, ValueError): + # Convert to string if serialization fails + serialized_dict[key] = str(value) # type: ignore + return serialized_dict diff --git a/package/tests/test_models/test_flowchart.py b/package/tests/test_models/test_flowchart.py index 521cc419f8..560e2e684b 100644 --- a/package/tests/test_models/test_flowchart.py +++ b/package/tests/test_models/test_flowchart.py @@ -215,6 +215,49 @@ def test_create_parameters_node_single_parameter( assert parameters_node.parameter_value == 0.3 assert parameters_node.modular_pipelines == expected_modular_pipelines + def test_create_single_parameter_with_complex_type(self): + parameters_dataset = MemoryDataset(data=object()) + parameters_node = GraphNode.create_parameters_node( + dataset_id="params:test_split_ratio", + dataset_name="params:test_split_ratio", + layer=None, + tags=set(), + parameters=parameters_dataset, + modular_pipelines=set(), + ) + assert isinstance(parameters_node, ParametersNode) + assert parameters_node.kedro_obj is parameters_dataset + assert not parameters_node.is_all_parameters() + assert parameters_node.is_single_parameter() + assert parameters_node.parameter_value == str(parameters_node.kedro_obj.load()) + + def test_create_all_parameters_with_complex_type(self): + parameters_dataset = MemoryDataset( + data={ + "test_split_ratio": 0.3, + "num_epochs": 1000, + "complex_param": object(), + } + ) + parameters_node = GraphNode.create_parameters_node( + dataset_id="parameters", + dataset_name="parameters", + layer=None, + tags=set(), + parameters=parameters_dataset, + modular_pipelines=set(), + ) + assert isinstance(parameters_node, ParametersNode) + assert parameters_node.kedro_obj is parameters_dataset + assert parameters_node.id == "parameters" + assert parameters_node.is_all_parameters() + assert not parameters_node.is_single_parameter() + assert parameters_node.parameter_value == { + "test_split_ratio": 0.3, + "num_epochs": 1000, + "complex_param": str(parameters_node.kedro_obj.load()["complex_param"]), + } + def test_create_non_existing_parameter_node(self): """Test the case where ``parameters`` is equal to None""" parameters_node = GraphNode.create_parameters_node( diff --git a/package/tests/test_models/test_utils.py b/package/tests/test_models/test_utils.py index 1d838a8c80..c0a0c897ee 100644 --- a/package/tests/test_models/test_utils.py +++ b/package/tests/test_models/test_utils.py @@ -1,7 +1,7 @@ import pytest from kedro.io import MemoryDataset -from kedro_viz.models.utils import get_dataset_type +from kedro_viz.models.utils import get_dataset_type, serialize_dict @pytest.mark.parametrize( @@ -10,3 +10,42 @@ ) def test_get_dataset_type(dataset, expected_type): assert get_dataset_type(dataset) == expected_type + + +def test_serialize_dict_with_serializable_values(): + original_dict = {"key1": "value1", "key2": 123, "key3": 45.67, "key4": 0.1} + expected_dict = {"key1": "value1", "key2": 123, "key3": 45.67, "key4": 0.1} + assert serialize_dict(original_dict) == expected_dict + + +def test_serialize_dict_with_non_serializable_values(): + original_dict = {"key1": "value1", "key2": object()} + expected_dict = {"key1": "value1", "key2": str(original_dict["key2"])} + assert serialize_dict(original_dict) == expected_dict + + +def test_serialize_dict_with_nested_dict(): + original_dict = { + "key1": "value1", + "key2": {"nested_key1": "nested_value1", "nested_key2": [object(), "value2"]}, + } + expected_dict = { + "key1": "value1", + "key2": { + "nested_key1": "nested_value1", + "nested_key2": str(original_dict["key2"]["nested_key2"]), + }, + } + assert serialize_dict(original_dict) == expected_dict + + +def test_serialize_dict_with_empty_dict(): + original_dict = {} + expected_dict = {} + assert serialize_dict(original_dict) == expected_dict + + +def test_serialize_dict_with_none_value(): + original_dict = {"key1": None} + expected_dict = {"key1": None} + assert serialize_dict(original_dict) == expected_dict diff --git a/src/components/export-modal/export-modal.js b/src/components/export-modal/export-modal.js old mode 100755 new mode 100644 diff --git a/src/components/flowchart/flowchart.js b/src/components/flowchart/flowchart.js old mode 100755 new mode 100644 diff --git a/src/components/flowchart/styles/_layers.scss b/src/components/flowchart/styles/_layers.scss old mode 100755 new mode 100644 diff --git a/src/components/global-toolbar/global-toolbar.js b/src/components/global-toolbar/global-toolbar.js old mode 100755 new mode 100644 diff --git a/src/components/ui/button/button.js b/src/components/ui/button/button.js old mode 100755 new mode 100644 diff --git a/src/components/ui/dropdown/dropdown.js b/src/components/ui/dropdown/dropdown.js old mode 100755 new mode 100644 diff --git a/src/components/update-reminder/update-reminder-content.js b/src/components/update-reminder/update-reminder-content.js old mode 100755 new mode 100644 diff --git a/src/config.js b/src/config.js old mode 100755 new mode 100644 diff --git a/src/store/initial-state.js b/src/store/initial-state.js old mode 100755 new mode 100644 diff --git a/tools/test-lib/react-app/app.js b/tools/test-lib/react-app/app.js old mode 100755 new mode 100644 From 2f82ea8a3696e0547fec82a717020799a032059e Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Wed, 2 Oct 2024 11:42:31 -0500 Subject: [PATCH 3/6] fix lint --- package/kedro_viz/api/apps.py | 3 +-- package/kedro_viz/data_access/repositories/catalog.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/package/kedro_viz/api/apps.py b/package/kedro_viz/api/apps.py index d5b5c535ca..aef4d44715 100644 --- a/package/kedro_viz/api/apps.py +++ b/package/kedro_viz/api/apps.py @@ -1,7 +1,6 @@ """`kedro_viz.api.app` defines the FastAPI app to serve Kedro data in a RESTful API. This data could either come from a real Kedro project or a file. """ - import json import os import time @@ -43,7 +42,7 @@ def _create_base_api_app() -> FastAPI: @app.middleware("http") async def set_secure_headers(request, call_next): response = await call_next(request) - secure_headers.framework.fastapi(response) + secure_headers.framework.fastapi(response) # pylint: disable=no-member return response return app diff --git a/package/kedro_viz/data_access/repositories/catalog.py b/package/kedro_viz/data_access/repositories/catalog.py index d9367cbeb1..8f6b104931 100644 --- a/package/kedro_viz/data_access/repositories/catalog.py +++ b/package/kedro_viz/data_access/repositories/catalog.py @@ -131,7 +131,7 @@ def get_dataset(self, dataset_name: str) -> Optional["AbstractDataset"]: else: # pragma: no cover dataset_obj = self._catalog._get_dataset(dataset_name) except DatasetNotFoundError: - dataset_obj = MemoryDataset() # type: ignore # pylint: disable=abstract-class-instantiated + dataset_obj = MemoryDataset() # type: ignore return dataset_obj From 53fc3cfa609adfeb4ef19bfedb40e605b86af17d Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Wed, 2 Oct 2024 11:54:21 -0500 Subject: [PATCH 4/6] fix lint --- package/kedro_viz/data_access/repositories/catalog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/kedro_viz/data_access/repositories/catalog.py b/package/kedro_viz/data_access/repositories/catalog.py index 8f6b104931..38d9a6772d 100644 --- a/package/kedro_viz/data_access/repositories/catalog.py +++ b/package/kedro_viz/data_access/repositories/catalog.py @@ -131,7 +131,7 @@ def get_dataset(self, dataset_name: str) -> Optional["AbstractDataset"]: else: # pragma: no cover dataset_obj = self._catalog._get_dataset(dataset_name) except DatasetNotFoundError: - dataset_obj = MemoryDataset() # type: ignore + dataset_obj = MemoryDataset() return dataset_obj From c86a851f8a6ee3bf58b37e6455bad3da738181c3 Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Wed, 2 Oct 2024 12:34:30 -0500 Subject: [PATCH 5/6] update release note --- RELEASE.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/RELEASE.md b/RELEASE.md index 72cfee8cdb..b93010273a 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -12,6 +12,9 @@ Please follow the established format: - Update Kedro-Viz telemetry for opt-out model (#2022) +## Bug fixes and other changes + +- Fix unserializable parameters value (#2122) # Release 10.0.0 From e0df2baa841fcf467be71e3ad5b35701b13628b3 Mon Sep 17 00:00:00 2001 From: ravi-kumar-pilla Date: Wed, 2 Oct 2024 14:50:42 -0500 Subject: [PATCH 6/6] use json_encoder --- package/kedro_viz/models/flowchart.py | 23 ++++++++---- package/kedro_viz/models/utils.py | 21 ----------- package/tests/test_models/test_flowchart.py | 13 +++---- package/tests/test_models/test_utils.py | 41 +-------------------- 4 files changed, 21 insertions(+), 77 deletions(-) diff --git a/package/kedro_viz/models/flowchart.py b/package/kedro_viz/models/flowchart.py index 65a208a6c8..8828650a7e 100644 --- a/package/kedro_viz/models/flowchart.py +++ b/package/kedro_viz/models/flowchart.py @@ -9,6 +9,7 @@ from types import FunctionType from typing import Any, ClassVar, Dict, List, Optional, Set, Union, cast +from fastapi.encoders import jsonable_encoder from kedro.pipeline.node import Node as KedroNode from pydantic import ( BaseModel, @@ -19,7 +20,7 @@ model_validator, ) -from kedro_viz.models.utils import get_dataset_type, serialize_dict +from kedro_viz.models.utils import get_dataset_type from kedro_viz.utils import TRANSCODING_SEPARATOR, _strip_transcoding try: @@ -862,13 +863,11 @@ def parameter_value(self) -> Any: try: actual_parameter_value = self.kedro_obj.load() - if self.is_all_parameters() and isinstance(actual_parameter_value, dict): - serialized_parameter_value = serialize_dict(actual_parameter_value) - return serialized_parameter_value - if isinstance(actual_parameter_value, (int, float)): - # handles a single parameter value which can be serialized - return actual_parameter_value - # handles any complex type that can't be serialized + # Return only json serializable value + return jsonable_encoder(actual_parameter_value) + except (TypeError, ValueError, RecursionError): + # In case the parameter is not JSON serializable, + # return the string representation return str(actual_parameter_value) except (AttributeError, DatasetError): # This except clause triggers if the user passes a parameter that is not @@ -878,6 +877,14 @@ def parameter_value(self) -> Any: "Cannot find parameter `%s` in the catalog.", self.parameter_name ) return None + # pylint: disable=broad-exception-caught + except Exception as exc: # pragma: no cover + logger.error( + "An error occurred when loading parameter `%s` in the catalog :: %s", + self.parameter_name, + exc, + ) + return None class ParametersNodeMetadata(GraphNodeMetadata): diff --git a/package/kedro_viz/models/utils.py b/package/kedro_viz/models/utils.py index c84c32c6a2..0024f13e22 100644 --- a/package/kedro_viz/models/utils.py +++ b/package/kedro_viz/models/utils.py @@ -1,6 +1,5 @@ """`kedro_viz.models.utils` contains utility functions used in the `kedro_viz.models` package""" -import json import logging from typing import TYPE_CHECKING @@ -45,23 +44,3 @@ def get_dataset_type(dataset: "AbstractDataset") -> str: abbreviated_module_name = ".".join(dataset.__class__.__module__.split(".")[-2:]) class_name = f"{dataset.__class__.__qualname__}" return f"{abbreviated_module_name}.{class_name}" - - -def serialize_dict(original_dict: dict) -> dict: - """Serialize a dictionary by converting its values to strings - if the value is non serializable.""" - serialized_dict = {} - - for key, value in original_dict.items(): - if isinstance(value, dict): - # Recursively process the nested dictionary - serialized_dict[key] = serialize_dict(value) - else: - try: - # Check if the value is serializable - json.dumps(value) - serialized_dict[key] = value - except (TypeError, ValueError): - # Convert to string if serialization fails - serialized_dict[key] = str(value) # type: ignore - return serialized_dict diff --git a/package/tests/test_models/test_flowchart.py b/package/tests/test_models/test_flowchart.py index 560e2e684b..eab5283817 100644 --- a/package/tests/test_models/test_flowchart.py +++ b/package/tests/test_models/test_flowchart.py @@ -1,7 +1,7 @@ from functools import partial from pathlib import Path from textwrap import dedent -from unittest.mock import call, patch +from unittest.mock import Mock, call, patch import pytest from kedro.io import MemoryDataset @@ -229,14 +229,15 @@ def test_create_single_parameter_with_complex_type(self): assert parameters_node.kedro_obj is parameters_dataset assert not parameters_node.is_all_parameters() assert parameters_node.is_single_parameter() - assert parameters_node.parameter_value == str(parameters_node.kedro_obj.load()) + assert isinstance(parameters_node.parameter_value, str) def test_create_all_parameters_with_complex_type(self): + mock_object = Mock() parameters_dataset = MemoryDataset( data={ "test_split_ratio": 0.3, "num_epochs": 1000, - "complex_param": object(), + "complex_param": mock_object, } ) parameters_node = GraphNode.create_parameters_node( @@ -252,11 +253,7 @@ def test_create_all_parameters_with_complex_type(self): assert parameters_node.id == "parameters" assert parameters_node.is_all_parameters() assert not parameters_node.is_single_parameter() - assert parameters_node.parameter_value == { - "test_split_ratio": 0.3, - "num_epochs": 1000, - "complex_param": str(parameters_node.kedro_obj.load()["complex_param"]), - } + assert isinstance(parameters_node.parameter_value, str) def test_create_non_existing_parameter_node(self): """Test the case where ``parameters`` is equal to None""" diff --git a/package/tests/test_models/test_utils.py b/package/tests/test_models/test_utils.py index c0a0c897ee..1d838a8c80 100644 --- a/package/tests/test_models/test_utils.py +++ b/package/tests/test_models/test_utils.py @@ -1,7 +1,7 @@ import pytest from kedro.io import MemoryDataset -from kedro_viz.models.utils import get_dataset_type, serialize_dict +from kedro_viz.models.utils import get_dataset_type @pytest.mark.parametrize( @@ -10,42 +10,3 @@ ) def test_get_dataset_type(dataset, expected_type): assert get_dataset_type(dataset) == expected_type - - -def test_serialize_dict_with_serializable_values(): - original_dict = {"key1": "value1", "key2": 123, "key3": 45.67, "key4": 0.1} - expected_dict = {"key1": "value1", "key2": 123, "key3": 45.67, "key4": 0.1} - assert serialize_dict(original_dict) == expected_dict - - -def test_serialize_dict_with_non_serializable_values(): - original_dict = {"key1": "value1", "key2": object()} - expected_dict = {"key1": "value1", "key2": str(original_dict["key2"])} - assert serialize_dict(original_dict) == expected_dict - - -def test_serialize_dict_with_nested_dict(): - original_dict = { - "key1": "value1", - "key2": {"nested_key1": "nested_value1", "nested_key2": [object(), "value2"]}, - } - expected_dict = { - "key1": "value1", - "key2": { - "nested_key1": "nested_value1", - "nested_key2": str(original_dict["key2"]["nested_key2"]), - }, - } - assert serialize_dict(original_dict) == expected_dict - - -def test_serialize_dict_with_empty_dict(): - original_dict = {} - expected_dict = {} - assert serialize_dict(original_dict) == expected_dict - - -def test_serialize_dict_with_none_value(): - original_dict = {"key1": None} - expected_dict = {"key1": None} - assert serialize_dict(original_dict) == expected_dict