From c03fbb0056a4e2907e2d5f15ea09af7fdd4bd748 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Fri, 20 Nov 2020 16:04:33 +0545 Subject: [PATCH] parametrization: raise appropriate error message on merge error (#4921) --- dvc/parsing/context.py | 28 +++++++++++++++++++++++++--- tests/unit/test_context.py | 6 +++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/dvc/parsing/context.py b/dvc/parsing/context.py index 9d147f576..87b71834b 100644 --- a/dvc/parsing/context.py +++ b/dvc/parsing/context.py @@ -9,6 +9,7 @@ from typing import Any, List, Optional, Union from funcy import identity, rpartial +from dvc.exceptions import DvcException from dvc.parsing.interpolate import ( get_expression, get_matches, @@ -21,15 +22,36 @@ from dvc.utils.serialize import LOADERS SeqOrMap = Union[Sequence, Mapping] +class ContextError(DvcException): + pass + + +class MergeError(ContextError): + def __init__(self, key, new, into): + self.key = key + if not isinstance(into[key], Node) or not isinstance(new, Node): + super().__init__( + f"cannot merge '{key}' as it already exists in {into}" + ) + return + + assert isinstance(new, Node) and isinstance(into[key], Node) + preexisting = into[key].meta.source + new_src = new.meta.source + path = new.meta.path() + super().__init__( + f"cannot redefine '{path}' from '{new_src}'" + f" as it already exists in '{preexisting}'" + ) + + def _merge(into, update, overwrite): for key, val in update.items(): if isinstance(into.get(key), Mapping) and isinstance(val, Mapping): _merge(into[key], val, overwrite) else: if key in into and not overwrite: - raise ValueError( - f"Cannot overwrite as key {key} already exists in {into}" - ) + raise MergeError(key, val, into) into[key] = val assert isinstance(into[key], Node) diff --git a/tests/unit/test_context.py b/tests/unit/test_context.py index 1c2053896..52be26b18 100644 --- a/tests/unit/test_context.py +++ b/tests/unit/test_context.py @@ -4,7 +4,7 @@ from math import pi import pytest from dvc.parsing import DEFAULT_PARAMS_FILE -from dvc.parsing.context import Context, CtxDict, CtxList, Value +from dvc.parsing.context import Context, CtxDict, CtxList, MergeError, Value from dvc.tree.local import LocalTree from dvc.utils.serialize import dump_yaml from tests.func.test_stage_resolver import recurse_not_a_node @@ -173,7 +173,7 @@ def test_merge_dict(): c1.merge_update(c2) assert c1.select("Train.us") == CtxDict(lr=10, layers=100) - with pytest.raises(ValueError): + with pytest.raises(MergeError): # cannot overwrite by default c1.merge_update({"Train": {"us": {"lr": 15}}}) @@ -187,7 +187,7 @@ def test_merge_dict(): def test_merge_list(): c1 = Context(lst=[1, 2, 3]) - with pytest.raises(ValueError): + with pytest.raises(MergeError): # cannot overwrite by default c1.merge_update({"lst": [10, 11, 12]})