using YAML merge tag (<<) results in exception

[o-smirnov]

Describe the bug

This YAML code causes an exception with ``OmegaConf.create()```:

OmegaConf(create("""
        a: &A
            x: 1
            y: 2

        b:
            <<: *A
            x: 3
            z: 1
"""))
# yaml.constructor.ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:merge'

It loads fine with the standard loader

To Reproduce
See above.

Expected behavior
Excpected to load without error.

Additional context

• OmegaConf version: master
• Python version: 3.6.9
• Operating system : Ubuntu 18.04
• Please provide a minimal repro

[omry]

@jgehring, this is related to your fix of #257, mind taking a look?

[jgehring]

Sorry for the delay -- I missed this notification. It seems like this can be hotfixed by adding a noop-constructor for merge tags like so:

diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py
index 1dcf7d9..588b4c8 100644
--- a/omegaconf/_utils.py
+++ b/omegaconf/_utils.py
@@ -129,6 +129,9 @@ def get_yaml_loader() -> Any:
     loader.add_constructor(
         yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, no_duplicates_constructor
     )
+    loader.add_constructor(
+        "tag:yaml.org,2002:merge", lambda loader, node, deep=False: None
+    )
     return loader

For the test case mentioned here, this produces:

a:
  x: 1
  'y': 2
b:
  x: 3
  'y': 2
  z: 1

This looks correct to me, i.e. the value for y is inserted while the value already defined for x is maintained, which is what's described in https://yaml.org/type/merge.html. I can do a PR for this, but admittedly I'm stepping out of familiar territory here and have limited knowledge about the exact mechanics of this fix.

[omry]

Thanks @jgehring.
I am even less familiar with the intricacies of pyyaml.
I think we can go in either:

1. Try to push forward and add support for yaml merges (with a unit test or two).
2. Backtrack and remove support for duplicate key validation (which is what introduced this problem

Since you are the one that asked (and added) duplicate key validation, I will let you make the call :)

[jgehring]

Ok, I did some more digging and came up with an IMHO reasonable solution (see PR). It's more or less what the PyYAML maintainer suggested at yaml/pyyaml#165 (comment).

[omry]

Given that Tina added a bug tag to it a few days ago, maybe they will actually address this in pyyaml eventually.