Make Model.reify() respect existing variables

... if given.

penman/model.py

@@ -11,14 +11,16 @@
 from penman.exceptions import ModelError
 from penman.types import (
     Identifier,
+    IdSet,
     Role,
     Constant,
     BasicTriple
 )
 
 
-_Reification = Tuple[Role, Constant, Role, Role]
+_ReificationSpec = Tuple[Role, Constant, Role, Role]
 _Reified = Tuple[Constant, Role, Role]
+_Reification = Tuple[BasicTriple, BasicTriple, BasicTriple]
 
 
 class Model(object):
@@ -41,7 +43,7 @@ def __init__(self,
                  nodetype_role: Role = ':instance',
                  roles: Mapping[Role, Any] = None,
                  normalizations: Mapping[Role, Role] = None,
-                 reifications: Iterable[_Reification] = None):
+                 reifications: Iterable[_ReificationSpec] = None):
         self.top_identifier = top_identifier
         self.top_role = top_role
         self.nodetype_role = nodetype_role
@@ -183,22 +185,39 @@ def is_reifiable(self, triple: BasicTriple) -> bool:
         """Return `True` if the role of *triple* can be reified."""
         return triple[1] in self.reifications
 
-    def reify(self, triple: BasicTriple) -> List[BasicTriple]:
+    def reify(self,
+              triple: BasicTriple,
+              variables: IdSet = None) -> _Reification:
         """
-        Return the list of triples that reify *triple*.
+        Return the three triples that reify *triple*.
 
-        Note that the node identifier for the reified node is not
-        necessarily valid for the target graph. When incorporating
-        the reified triples, this identifier should be replaced.
+        Note that, unless *variables* is given, the node identifier
+        for the reified node is not necessarily valid for the target
+        graph. When incorporating the reified triples, this identifier
+        should then be replaced.
 
         If the role of *triple* does not have a defined reification, a
         :exc:`ModelError` is raised.
+
+        Args:
+            triple: the triple to reify
+            variables: a set of variables that should not be used for
+                the reified node's variable
+        Returns:
+            The 3-tuple of triples that reify *triple*.
         """
         source, role, target = triple
         if role not in self.reifications:
             raise ModelError("'{}' cannot be reified".format(role))
         label, source_role, target_role = next(iter(self.reifications[role]))
-        dummy_id = '_'
-        return [(source, self.invert_role(source_role), dummy_id),
-                (dummy_id, self.nodetype_role, label),
-                (dummy_id, target_role, target)]
+
+        var = '_'
+        if variables:
+            i = 2
+            while var in variables:
+                var = '_{}'.format(i)
+                i += 1
+
+        return ((source, self.invert_role(source_role), var),
+                (var, self.nodetype_role, label),
+                (var, target_role, target))

tests/conftest.py

@@ -1,6 +1,30 @@
 
 import pytest
 
+
+@pytest.fixture(scope='module')
+def mini_amr():
+    return {
+        'roles': {
+            ':ARG0': {'type': 'frame'},
+            ':ARG1': {'type': 'frame'},
+            ":accompanier": {"type": "general"},
+            ":domain": {"type": "general"},
+            ":consist-of": {"type": "general"},
+            ":mod": {"type": "general"},
+            ":op[0-9]+": {"type": "op"},
+        },
+        'normalizations': {
+            ':mod-of': ':domain',
+            ':domain-of': ':mod',
+        },
+        'reifications': [
+            (':accompanier', 'accompany-01', ':ARG0', ':ARG1'),
+	    (':mod', 'have-mod-91', ':ARG1', ':ARG2')
+        ]
+    }
+
+
 @pytest.fixture
 def x1():
     return (
@@ -23,6 +47,7 @@ def x1():
         ]
     )
 
+
 @pytest.fixture
 def isi_aligned():
     """He drives carelessly."""

tests/test_model.py

@@ -4,41 +4,22 @@
 from penman.exceptions import ModelError
 from penman.model import Model
 
-mini_amr = {
-    'roles': {
-        ':ARG0': {'type': 'frame'},
-        ':ARG1': {'type': 'frame'},
-        ":accompanier": {"type": "general"},
-        ":domain": {"type": "general"},
-        ":consist-of": {"type": "general"},
-        ":mod": {"type": "general"},
-        ":op[0-9]+": {"type": "op"},
-    },
-    'normalizations': {
-        ':mod-of': ':domain',
-        ':domain-of': ':mod',
-    },
-    'reifications': [
-        (':accompanier', 'accompany-01', ':ARG0', ':ARG1'),
-	(':mod', 'have-mod-91', ':ARG1', ':ARG2')
-    ]
-}
 
 
 class TestModel:
-    def test__init__(self):
+    def test__init__(self, mini_amr):
         m = Model()
         assert len(m.roles) == 0
         m = Model(roles=mini_amr['roles'])
         assert len(m.roles) == 7
 
-    def test_from_dict(self):
+    def test_from_dict(self, mini_amr):
         assert Model.from_dict(mini_amr) == Model(
             roles=mini_amr['roles'],
             normalizations=mini_amr['normalizations'],
             reifications=mini_amr['reifications'])
 
-    def test_has_role(self):
+    def test_has_role(self, mini_amr):
         m = Model()
         assert not m.has_role('')
         assert m.has_role(m.nodetype_role)
@@ -60,7 +41,7 @@ def test_has_role(self):
         assert m.has_role(':op9999')
         assert not m.has_role(':op[0-9]+')
 
-    def test_is_role_inverted(self):
+    def test_is_role_inverted(self, mini_amr):
         m = Model()
         assert m.is_role_inverted(':ARG0-of')
         assert m.is_role_inverted(':-of')
@@ -83,7 +64,7 @@ def test_is_role_inverted(self):
         # assert not m.is_role_inverted('mod')
         # assert not m.is_role_inverted('consist-of')
 
-    def test_invert_role(self):
+    def test_invert_role(self, mini_amr):
         m = Model()
         assert m.invert_role(':ARG0')       == ':ARG0-of'
         assert m.invert_role(':ARG0-of')    == ':ARG0'
@@ -104,7 +85,7 @@ def test_invert_role(self):
         # assert m.invert_role('mod')        == 'domain'
         # assert m.invert_role('domain')     == 'mod'
 
-    def test_invert(self):
+    def test_invert(self, mini_amr):
         m = Model()
         assert m.invert(('a', ':ARG0', 'b'))       == ('b', ':ARG0-of', 'a')
         assert m.invert(('a', ':ARG0-of', 'b'))    == ('b', ':ARG0', 'a')
@@ -126,7 +107,7 @@ def test_invert(self):
         # assert m.invert(('a', 'domain', 'b'))      == ('b', 'mod', 'a')
 
 
-    def test_deinvert(self):
+    def test_deinvert(self, mini_amr):
         m = Model()
         assert m.deinvert(('a', ':ARG0', 'b'))       == ('a', ':ARG0', 'b')
         assert m.deinvert(('a', ':ARG0-of', 'b'))    == ('b', ':ARG0', 'a')
@@ -147,7 +128,7 @@ def test_deinvert(self):
         # assert m.deinvert(('a', 'ARG0-of', 'b'))     == ('b', 'ARG0', 'a')
         # assert m.deinvert(('a', 'consist-of', 'b'))  == ('a', 'consist-of', 'b')
 
-    def test_canonicalize_role(self):
+    def test_canonicalize_role(self, mini_amr):
         m = Model()
         assert m.canonicalize_role(':ARG0')          == ':ARG0'
         assert m.canonicalize_role(':ARG0-of')       == ':ARG0-of'
@@ -180,7 +161,7 @@ def test_canonicalize_role(self):
         assert m.canonicalize_role('consist-of')     == ':consist-of'
         assert m.canonicalize_role('consist-of-of')  == ':consist-of-of'
 
-    def test_canonicalize(self):
+    def test_canonicalize(self, mini_amr):
         m = Model()
         assert m.canonicalize(('a', ':ARG0', 'b'))          == ('a', ':ARG0', 'b')
         assert m.canonicalize(('a', ':ARG0-of', 'b'))       == ('a', ':ARG0-of', 'b')
@@ -213,7 +194,7 @@ def test_canonicalize(self):
         assert m.canonicalize(('a', 'consist-of', 'b'))     == ('a', ':consist-of', 'b')
         assert m.canonicalize(('a', 'consist-of-of', 'b'))  == ('a', ':consist-of-of', 'b')
 
-    def test_is_reifiable(self):
+    def test_is_reifiable(self, mini_amr):
         m = Model()
         assert not m.is_reifiable(('a', ':ARG0', 'b'))
         assert not m.is_reifiable(('a', ':accompanier', 'b'))
@@ -225,7 +206,7 @@ def test_is_reifiable(self):
         assert not m.is_reifiable(('a', ':domain', 'b'))
         assert m.is_reifiable(('a', ':mod', 'b'))
 
-    def test_reify(self):
+    def test_reify(self, mini_amr):
         m = Model()
         with pytest.raises(ModelError):
             m.reify(('a', ':ARG0', 'b'))
@@ -238,13 +219,18 @@ def test_reify(self):
         m = Model.from_dict(mini_amr)
         with pytest.raises(ModelError):
             m.reify(('a', ':ARG0', 'b'))
-        assert m.reify(('a', ':accompanier', 'b')) == [
+        assert m.reify(('a', ':accompanier', 'b')) == (
             ('a', ':ARG0-of', '_'),
             ('_', ':instance', 'accompany-01'),
-            ('_', ':ARG1', 'b')]
+            ('_', ':ARG1', 'b'))
         with pytest.raises(ModelError):
             assert m.reify(('a', ':domain', 'b'))
-        assert m.reify(('a', ':mod', 'b')) == [
+        assert m.reify(('a', ':mod', 'b')) == (
             ('a', ':ARG1-of', '_'),
             ('_', ':instance', 'have-mod-91'),
-            ('_', ':ARG2', 'b')]
+            ('_', ':ARG2', 'b'))
+        # ensure unique ids if variables is specified
+        assert m.reify(('a', ':mod', 'b'), variables={'a', 'b', '_'}) == (
+            ('a', ':ARG1-of', '_2'),
+            ('_2', ':instance', 'have-mod-91'),
+            ('_2', ':ARG2', 'b'))