core[patch]: dont mutate merged lists/dicts (#25858)

Update merging utils to
- not mutate objects
- have special handling to 'type' keys in dicts
This commit is contained in:
Bagatur 2024-08-29 13:34:54 -07:00 committed by GitHub
parent 09c2d8faca
commit fabd3295fa
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 54 additions and 4 deletions

View File

@ -41,6 +41,19 @@ def merge_dicts(left: Dict[str, Any], *others: Dict[str, Any]) -> Dict[str, Any]
" but with a different type."
)
elif isinstance(merged[right_k], str):
# TODO: Add below special handling for 'type' key in 0.3 and remove
# merge_lists 'type' logic.
#
# if right_k == "type":
# if merged[right_k] == right_v:
# continue
# else:
# raise ValueError(
# "Unable to merge. Two different values seen for special "
# f"key 'type': {merged[right_k]} and {right_v}. 'type' "
# "should either occur once or have the same value across "
# "all dicts."
# )
merged[right_k] += right_v
elif isinstance(merged[right_k], dict):
merged[right_k] = merge_dicts(merged[right_k], right_v)
@ -81,10 +94,10 @@ def merge_lists(left: Optional[List], *others: Optional[List]) -> Optional[List]
if e_left["index"] == e["index"]
]
if to_merge:
# If a top-level "type" has been set for a chunk, it should no
# longer be overridden by the "type" field in future chunks.
if "type" in merged[to_merge[0]] and "type" in e:
e.pop("type")
# TODO: Remove this once merge_dict is updated with special
# handling for 'type'.
if "type" in e:
e = {k: v for k, v in e.items() if k != "type"}
merged[to_merge[0]] = merge_dicts(merged[to_merge[0]], e)
else:
merged.append(e)

View File

@ -1,6 +1,7 @@
import os
import re
from contextlib import AbstractContextManager, nullcontext
from copy import deepcopy
from typing import Any, Callable, Dict, Optional, Tuple, Type, Union
from unittest.mock import patch
@ -120,9 +121,45 @@ def test_merge_dicts(
else:
err = nullcontext()
left_copy = deepcopy(left)
right_copy = deepcopy(right)
with err:
actual = merge_dicts(left, right)
assert actual == expected
# no mutation
assert left == left_copy
assert right == right_copy
@pytest.mark.parametrize(
("left", "right", "expected"),
(
# 'type' special key handling
({"type": "foo"}, {"type": "foo"}, {"type": "foo"}),
(
{"type": "foo"},
{"type": "bar"},
pytest.raises(ValueError, match="Unable to merge."),
),
),
)
@pytest.mark.xfail(reason="Refactors to make in 0.3")
def test_merge_dicts_0_3(
left: dict, right: dict, expected: Union[dict, AbstractContextManager]
) -> None:
if isinstance(expected, AbstractContextManager):
err = expected
else:
err = nullcontext()
left_copy = deepcopy(left)
right_copy = deepcopy(right)
with err:
actual = merge_dicts(left, right)
assert actual == expected
# no mutation
assert left == left_copy
assert right == right_copy
@pytest.mark.parametrize(