Repository: iterative/dvc

Issue:
tests: test_experiments::test_untracked is failing in CI
may be related to existing flaky pygit tests issue?

Discussion:

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9da1f7712..fad2beb13 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -18,7 +18,7 @@ repos:
       - id: isort
         language_version: python3
     repo: https://github.com/timothycrosley/isort
-    rev: 5.9.2
+    rev: 5.9.3
   - hooks:
       - id: flake8
         language_version: python3
diff --git a/dvc/config.py b/dvc/config.py
index a9d8b328d..98fe3086f 100644
--- a/dvc/config.py
+++ b/dvc/config.py
@@ -20,7 +20,15 @@ class ConfigError(DvcException):
         super().__init__(f"config file error: {msg}")
 
 
-class NoRemoteError(ConfigError):
+class RemoteConfigError(ConfigError):
+    pass
+
+
+class NoRemoteError(RemoteConfigError):
+    pass
+
+
+class RemoteNotFoundError(RemoteConfigError):
     pass
 
 
diff --git a/dvc/fs/__init__.py b/dvc/fs/__init__.py
index 117247d35..4aee14a9b 100644
--- a/dvc/fs/__init__.py
+++ b/dvc/fs/__init__.py
@@ -40,7 +40,12 @@ def get_fs_cls(remote_conf):
 def get_fs_config(config, **kwargs):
     name = kwargs.get("name")
     if name:
-        remote_conf = config["remote"][name.lower()]
+        try:
+            remote_conf = config["remote"][name.lower()]
+        except KeyError:
+            from dvc.config import RemoteNotFoundError
+
+            raise RemoteNotFoundError(f"remote '{name}' doesn't exist")
     else:
         remote_conf = kwargs
     return _resolve_remote_refs(config, remote_conf)
diff --git a/dvc/repo/experiments/__init__.py b/dvc/repo/experiments/__init__.py
index 5a560c14c..06f326d9e 100644
--- a/dvc/repo/experiments/__init__.py
+++ b/dvc/repo/experiments/__init__.py
@@ -352,8 +352,7 @@ class Experiments:
 
     def _update_params(self, params: dict):
         """Update experiment params files with the specified values."""
-        from benedict import benedict
-
+        from dvc.utils.collections import merge_params
         from dvc.utils.serialize import MODIFIERS
 
         logger.debug("Using experiment params '%s'", params)
@@ -363,7 +362,7 @@ class Experiments:
             suffix = path.suffix.lower()
             modify_data = MODIFIERS[suffix]
             with modify_data(path, fs=self.repo.fs) as data:
-                benedict(data).merge(params[params_fname], overwrite=True)
+                merge_params(data, params[params_fname])
 
         # Force params file changes to be staged in git
         # Otherwise in certain situations the changes to params file may be
diff --git a/dvc/utils/collections.py b/dvc/utils/collections.py
index 70ecf56e2..15363180d 100644
--- a/dvc/utils/collections.py
+++ b/dvc/utils/collections.py
@@ -80,6 +80,20 @@ def chunk_dict(d: Dict[_KT, _VT], size: int = 1) -> List[Dict[_KT, _VT]]:
     return [{key: d[key] for key in chunk} for chunk in chunks(size, d)]
 
 
+def merge_params(src: Dict, to_update: Dict) -> Dict:
+    """Recursively merges params with benedict's syntax support in-place."""
+    from benedict import benedict
+
+    if src:
+        benedict(src).merge(to_update, overwrite=True)
+    else:
+        # benedict has issues keeping references to an empty dictionary
+        # see: https://github.com/iterative/dvc/issues/6374.
+        # Also, passing to_update through benedict to expand the syntax.
+        src.update(benedict(to_update))
+    return src
+
+
 class _NamespacedDict(dict):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
diff --git a/setup.py b/setup.py
index ddf39a746..67f17a8f3 100644
--- a/setup.py
+++ b/setup.py
@@ -85,7 +85,7 @@ install_requires = [
     "shtab>=1.3.4,<2",
     "rich>=10.0.0",
     "dictdiffer>=0.8.1",
-    "python-benedict>=0.21.1,<0.24.1",
+    "python-benedict>=0.21.1",
     "pyparsing==2.4.7",
     "typing_extensions>=3.7.4",
     "fsspec>=2021.7.0",


Test patch:
diff --git a/tests/unit/fs/test_fs.py b/tests/unit/fs/test_fs.py
index 511486ed1..f1d82f064 100644
--- a/tests/unit/fs/test_fs.py
+++ b/tests/unit/fs/test_fs.py
@@ -1,6 +1,7 @@
 import pytest
 
-from dvc.fs import get_fs_cls
+from dvc.config import RemoteNotFoundError
+from dvc.fs import get_fs_cls, get_fs_config
 from dvc.fs.hdfs import HDFSFileSystem
 from dvc.fs.http import HTTPFileSystem
 from dvc.fs.https import HTTPSFileSystem
@@ -29,3 +30,8 @@ from dvc.fs.ssh import SSHFileSystem
 )
 def test_get_fs_cls(url, cls):
     assert get_fs_cls({"url": url}) == cls
+
+
+def test_get_fs_config():
+    with pytest.raises(RemoteNotFoundError):
+        get_fs_config({"remote": {}}, name="myremote")
diff --git a/tests/unit/utils/test_collections.py b/tests/unit/utils/test_collections.py
index 9de4b1917..f7b02d1c1 100644
--- a/tests/unit/utils/test_collections.py
+++ b/tests/unit/utils/test_collections.py
@@ -1,7 +1,13 @@
 # pylint: disable=unidiomatic-typecheck
+import pytest
 from mock import create_autospec
 
-from dvc.utils.collections import apply_diff, chunk_dict, validate
+from dvc.utils.collections import (
+    apply_diff,
+    chunk_dict,
+    merge_params,
+    validate,
+)
 
 
 class MyDict(dict):
@@ -135,3 +141,71 @@ def test_post_validate_decorator(mocker):
     result = func()
     test_func.assert_called_once()
     assert result == [1, 2]
+
+
+@pytest.mark.parametrize(
+    "changes, expected",
+    [
+        [{"foo": "baz"}, {"foo": "baz", "goo": {"bag": 3}, "lorem": False}],
+        [
+            {"foo": "baz", "goo": "bar"},
+            {"foo": "baz", "goo": "bar", "lorem": False},
+        ],
+        [
+            {"goo.bag": 4},
+            {"foo": {"bar": 1, "baz": 2}, "goo": {"bag": 4}, "lorem": False},
+        ],
+        [
+            {"foo[0]": "bar"},
+            {
+                "foo": {"bar": 1, "baz": 2, 0: "bar"},
+                "goo": {"bag": 3},
+                "lorem": False,
+            },
+        ],
+        [
+            {"foo[1].baz": 3},
+            {
+                "foo": {"bar": 1, "baz": 2, 1: {"baz": 3}},
+                "goo": {"bag": 3},
+                "lorem": False,
+            },
+        ],
+        [
+            {"foo[1]": ["baz", "goo"]},
+            {
+                "foo": {"bar": 1, "baz": 2, 1: ["baz", "goo"]},
+                "goo": {"bag": 3},
+                "lorem": False,
+            },
+        ],
+        [
+            {"lorem.ipsum": 3},
+            {
+                "foo": {"bar": 1, "baz": 2},
+                "goo": {"bag": 3},
+                "lorem": {"ipsum": 3},
+            },
+        ],
+        [{}, {"foo": {"bar": 1, "baz": 2}, "goo": {"bag": 3}, "lorem": False}],
+    ],
+)
+def test_merge_params(changes, expected):
+    params = {"foo": {"bar": 1, "baz": 2}, "goo": {"bag": 3}, "lorem": False}
+    merged = merge_params(params, changes)
+    assert merged == expected == params
+    assert params is merged  # references should be preserved
+
+
+@pytest.mark.parametrize(
+    "changes, expected",
+    [
+        [{"foo": "baz"}, {"foo": "baz"}],
+        [{"foo": "baz", "goo": "bar"}, {"foo": "baz", "goo": "bar"}],
+    ],
+)
+def test_merge_params_on_empty_src(changes, expected):
+    params = {}
+    merged = merge_params(params, changes)
+    assert merged == expected == params
+    assert params is merged  # references should be preserved