From e0e2c677c0284243b2e5acb37e7f7e6fd31f7768 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 13 Feb 2024 16:02:27 +0600 Subject: [PATCH 1/4] Add merge_dicts() tests --- .../core/tests/unit_tests/utils/test_utils.py | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/libs/core/tests/unit_tests/utils/test_utils.py b/libs/core/tests/unit_tests/utils/test_utils.py index e6dedb2a5a86bf..fb4341cc96ca71 100644 --- a/libs/core/tests/unit_tests/utils/test_utils.py +++ b/libs/core/tests/unit_tests/utils/test_utils.py @@ -4,6 +4,7 @@ import pytest from langchain_core.utils import check_package_version +from langchain_core.utils._merge import merge_dicts @pytest.mark.parametrize( @@ -28,3 +29,36 @@ def test_check_package_version( else: with pytest.raises(expected[0], match=expected[1]): check_package_version(package, **check_kwargs) + + +@pytest.mark.parametrize( + ("left", "right", "expected"), + ( + # Merge `None` and `1`. + ({"a": None}, {"a": 1}, {"a": 1}), + # Merge `1` and `None`. + ({"a": 1}, {"a": None}, {"a": 1}), + # Merge equal values. + ({"a": 1}, {"a": 1}, {"a": 1}), + ({"a": 1.5}, {"a": 1.5}, {"a": 1.5}), + ({"a": "txt"}, {"a": "txt"}, {"a": "txt"}), + ({"a": [1, 2]}, {"a": [1, 2]}, {"a": [1, 2]}), + ({"a": {"b": "txt"}}, {"a": {"b": "txt"}}, {"a": {"b": "txt"}}), + # Merge strings. + ({"a": "one"}, {"a": "two"}, {"a": "onetwo"}), + # Merge dicts. + ({"a": {"b": 1}}, {"a": {"c": 2}}, {"a": {"b": 1, "c": 2}}), + ( + {"function_call": {"arguments": None}}, + {"function_call": {"arguments": "{\n"}}, + {"function_call": {"arguments": "{\n"}}, + ), + # Merge lists. + ({"a": [1, 2]}, {"a": [3]}, {"a": [1, 2, 3]}), + ({"a": 1, "b": 2}, {"a": 1}, {"a": 1, "b": 2}), + ({"a": 1, "b": 2}, {"c": None}, {"a": 1, "b": 2, "c": None}), + ), +) +def test_merge_dicts(left: dict, right: dict, expected: dict) -> None: + actual = merge_dicts(left, right) + assert actual == expected From 1ff04d67299cc65409b7f7521dbda90f4d3b5ac3 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 13 Feb 2024 16:48:20 +0600 Subject: [PATCH 2/4] Improve None values processing in merge_dicts(), add tests --- libs/core/langchain_core/utils/_merge.py | 6 +-- .../core/tests/unit_tests/utils/test_utils.py | 45 +++++++++++++++++-- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/libs/core/langchain_core/utils/_merge.py b/libs/core/langchain_core/utils/_merge.py index e21fdd96621fe1..13b91270c70d8a 100644 --- a/libs/core/langchain_core/utils/_merge.py +++ b/libs/core/langchain_core/utils/_merge.py @@ -19,11 +19,9 @@ def merge_dicts(left: Dict[str, Any], right: Dict[str, Any]) -> Dict[str, Any]: for k, v in right.items(): if k not in merged: merged[k] = v - elif merged[k] is None and v: + elif v is not None and merged[k] is None: merged[k] = v - elif v is None: - continue - elif merged[k] == v: + elif v is None or merged[k] == v: continue elif type(merged[k]) != type(v): raise TypeError( diff --git a/libs/core/tests/unit_tests/utils/test_utils.py b/libs/core/tests/unit_tests/utils/test_utils.py index fb4341cc96ca71..a55a533bf413d8 100644 --- a/libs/core/tests/unit_tests/utils/test_utils.py +++ b/libs/core/tests/unit_tests/utils/test_utils.py @@ -1,3 +1,4 @@ +from contextlib import AbstractContextManager, nullcontext from typing import Dict, Optional, Tuple, Type from unittest.mock import patch @@ -38,9 +39,14 @@ def test_check_package_version( ({"a": None}, {"a": 1}, {"a": 1}), # Merge `1` and `None`. ({"a": 1}, {"a": None}, {"a": 1}), + # Merge `None` and a value. + ({"a": None}, {"a": 0}, {"a": 0}), + ({"a": None}, {"a": "txt"}, {"a": "txt"}), # Merge equal values. ({"a": 1}, {"a": 1}, {"a": 1}), ({"a": 1.5}, {"a": 1.5}, {"a": 1.5}), + ({"a": True}, {"a": True}, {"a": True}), + ({"a": False}, {"a": False}, {"a": False}), ({"a": "txt"}, {"a": "txt"}, {"a": "txt"}), ({"a": [1, 2]}, {"a": [1, 2]}, {"a": [1, 2]}), ({"a": {"b": "txt"}}, {"a": {"b": "txt"}}, {"a": {"b": "txt"}}), @@ -57,8 +63,41 @@ def test_check_package_version( ({"a": [1, 2]}, {"a": [3]}, {"a": [1, 2, 3]}), ({"a": 1, "b": 2}, {"a": 1}, {"a": 1, "b": 2}), ({"a": 1, "b": 2}, {"c": None}, {"a": 1, "b": 2, "c": None}), + # + # Invalid inputs. + # + ( + {"a": 1}, + {"a": "1"}, + pytest.raises( + TypeError, + match=( + 'additional_kwargs\["a"\] already exists in this message, ' + "but with a different type\." + ), + ), + ), + ( + {"a": (1, 2)}, + {"a": (3,)}, + pytest.raises( + TypeError, + match=( + "Additional kwargs key a already exists in left dict and value " + "has unsupported type .+tuple.+\." + ), + ), + ), ), ) -def test_merge_dicts(left: dict, right: dict, expected: dict) -> None: - actual = merge_dicts(left, right) - assert actual == expected +def test_merge_dicts( + left: dict, right: dict, expected: dict | AbstractContextManager +) -> None: + if isinstance(expected, AbstractContextManager): + err = expected + else: + err = nullcontext() + + with err: + actual = merge_dicts(left, right) + assert actual == expected From 6dd95a9e2317b1ed7bd7320ecbb4066e0eef9880 Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 13 Feb 2024 19:23:54 +0600 Subject: [PATCH 3/4] Make type hint py>=3.8 compatible --- libs/core/tests/unit_tests/utils/test_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/core/tests/unit_tests/utils/test_utils.py b/libs/core/tests/unit_tests/utils/test_utils.py index a55a533bf413d8..71df02f1e771a6 100644 --- a/libs/core/tests/unit_tests/utils/test_utils.py +++ b/libs/core/tests/unit_tests/utils/test_utils.py @@ -1,5 +1,5 @@ from contextlib import AbstractContextManager, nullcontext -from typing import Dict, Optional, Tuple, Type +from typing import Dict, Optional, Tuple, Type, Union from unittest.mock import patch import pytest @@ -91,7 +91,7 @@ def test_check_package_version( ), ) def test_merge_dicts( - left: dict, right: dict, expected: dict | AbstractContextManager + left: dict, right: dict, expected: Union[dict, AbstractContextManager] ) -> None: if isinstance(expected, AbstractContextManager): err = expected From 8bbbe950660e9e2c3aa23b52d46368c976d4ebda Mon Sep 17 00:00:00 2001 From: Sergey Kozlov Date: Tue, 13 Feb 2024 19:24:22 +0600 Subject: [PATCH 4/4] Fix deprecation warnings --- libs/core/tests/unit_tests/utils/test_utils.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libs/core/tests/unit_tests/utils/test_utils.py b/libs/core/tests/unit_tests/utils/test_utils.py index 71df02f1e771a6..a69ea2510b41fa 100644 --- a/libs/core/tests/unit_tests/utils/test_utils.py +++ b/libs/core/tests/unit_tests/utils/test_utils.py @@ -1,3 +1,4 @@ +import re from contextlib import AbstractContextManager, nullcontext from typing import Dict, Optional, Tuple, Type, Union from unittest.mock import patch @@ -71,9 +72,9 @@ def test_check_package_version( {"a": "1"}, pytest.raises( TypeError, - match=( - 'additional_kwargs\["a"\] already exists in this message, ' - "but with a different type\." + match=re.escape( + 'additional_kwargs["a"] already exists in this message, ' + "but with a different type." ), ), ), @@ -84,7 +85,7 @@ def test_check_package_version( TypeError, match=( "Additional kwargs key a already exists in left dict and value " - "has unsupported type .+tuple.+\." + "has unsupported type .+tuple.+." ), ), ),