Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add containment for trees and graphs #118

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion penman/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
Data structures for Penman graphs and triples.
"""

from typing import (Union, Optional, Mapping, List, Dict, Set, NamedTuple)
from typing import (Union, Optional, Mapping, List, Dict, Set, NamedTuple, Tuple)
from collections import defaultdict
import copy

Expand Down Expand Up @@ -173,6 +173,20 @@ def __isub__(self, other):
else:
return NotImplemented

def __contains__(self, item: Union[str, Triple, Tuple[str, str, str]]):
"""
Containment checking of either a string or a triple (tuple). In the case of a
string, will return True if the given string is a terminal instance in the graph.
If a triple or tuple, will return True if the triple exists in self.triples.
"""
if isinstance(item, str):
terminals = [t[2] for t in self.instances()]
return item in terminals
elif isinstance(item, tuple):
return item in self.triples
else:
raise NotImplemented

@property
def top(self) -> Union[Variable, None]:
"""
Expand Down
10 changes: 10 additions & 0 deletions penman/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,16 @@ def __str__(self) -> str:
s = _format(self.node, 2)
return f'Tree(\n {s})'

def __contains__(self, item: str):
"""
Test whether a given string is part of the tree as a terminal instance node.
"""
if isinstance(item, str):
terminals = [target for _, (role, target) in self.walk() if role == "/" and is_atomic(target)]
return item in terminals
else:
raise NotImplemented
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you could look for branches as you did for triples above. Otherwise, same comments as before:

  • make a set
  • Try not to raise a specific error, or if it's an unsupported type use TypeError


def nodes(self) -> List[Node]:
"""
Return the nodes in the tree as a flat list.
Expand Down
12 changes: 12 additions & 0 deletions tests/test_graph.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-

import penman
from penman import Triple

Graph = penman.Graph

Expand Down Expand Up @@ -102,6 +103,17 @@ def test__isub__(self):
assert g.top == 'b'
assert g is original

def test__contains__(self):
g = Graph([('a', ':instance', 'alpha'),
('a', ':ARG', 'b'),
('b', ':instance', 'beta')])

assert "alpha" in g
assert "beta" in g
assert "b" not in g
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is confusing because b is in g, it's just not defined as such. Not sure what the best path forward is. But you could have something obviously false, like assert "gamma" not in g

assert ('a', ':ARG', 'b') in g
assert Triple('b', ':instance', 'beta') in g

def test_top(self, x1):
assert Graph([('a', ':instance', None)]).top == 'a'
assert Graph(
Expand Down
10 changes: 8 additions & 2 deletions tests/test_tree.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import pytest

from penman import tree
Expand Down Expand Up @@ -47,6 +46,14 @@ def test__init__(self, empty_node, simple_node):
assert t.node == simple_node
assert t.metadata == {'snt': 'Alpha.'}

def test__contains__(self):
t = tree.Tree(('a', [('/', 'alpha'), (':ARG', ('b', [('/', 'beta')]))]))

assert "alpha" in t
assert "beta" in t
assert "b" not in t
assert ":ARG" not in t

def test_nodes(self, one_arg_node, reentrant):
t = tree.Tree(one_arg_node)
assert t.nodes() == [one_arg_node, ('b', [('/', 'beta')])]
Expand Down Expand Up @@ -86,7 +93,6 @@ def test_walk(self, one_arg_node, reentrant):
]

def test_reset_variables(self, one_arg_node, reentrant, var_instance):

def _vars(t):
return [v for v, _ in t.nodes()]

Expand Down