Repository: EmilStenstrom/conllu

Issue:
TokenList could inherit "list"
Hey. I have found this library quite useful. There is a suggestion that I ran into: `TokenList` could maybe inherit `list`, and so be treated like that.

I tried manipulating the sentences (TokenLists) as if they were lists, such as using `len` but then I saw they weren't. I thought about adding `__len__`, but I think it's more appropriate to make them lists, so they natively support other things such as adding them, comparing them, and so on.

What do you think guys?

Discussion:
EmilStenstrom: This looks great! The coverage and flake8 tests seem to fail on Travis. Do you get the same errors locally? 

Summary:
* There's a test missing for "models.py line 30->exit"
* There's a coverage error for "conllu/models.py:24:16: E126 continuation line over-indented for hanging indent"

Mind fixing those two? Thanks again!
bryant1410: That's weird, tests were passing locally on Python 2.7 and 3.6 and coverage
was 100%. I will check it out later.

On Thu, Sep 13, 2018, 3:46 PM Emil Stenström <notifications@github.com>
wrote:

> This looks great! The coverage and flake8 tests seem to fail on Travis. Do
> you get the same errors locally?
>
> Summary:
>
>    - There's a test missing for "models.py line 30->exit"
>    - There's a coverage error for "conllu/models.py:24:16: E126
>    continuation line over-indented for hanging indent"
>
> Mind fixing those two? Thanks again!
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/EmilStenstrom/conllu/pull/19#issuecomment-421129685>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ADuX3cIVLa4OnW6z7LYsyAqk75RvA4sPks5uarXxgaJpZM4WoFFX>
> .
>

EmilStenstrom: This looks fantastic. I'll merge and release a new version to PyPI!

Patch:
diff --git a/conllu/models.py b/conllu/models.py
index d57a49a..cd98ae4 100644
--- a/conllu/models.py
+++ b/conllu/models.py
@@ -5,25 +5,53 @@ from conllu.parser import ParseException, head_to_token, serialize
 
 DEFAULT_EXCLUDE_FIELDS = ('id', 'deprel', 'xpostag', 'feats', 'head', 'deps', 'misc')
 
-class TokenList(object):
-    tokens = None
+
+class TokenList(list):
     metadata = None
 
     def __init__(self, tokens, metadata=None):
+        super(TokenList, self).__init__(tokens)
         if not isinstance(tokens, list):
             raise ParseException("Can't create TokenList, tokens is not a list.")
 
-        self.tokens = tokens
         self.metadata = metadata
 
-    def __getitem__(self, key):
-        return self.tokens[key]
-
     def __repr__(self):
-        return 'TokenList<' + ', '.join([token['form'] for token in self.tokens]) + '>'
+        return 'TokenList<' + ', '.join(token['form'] for token in self) + '>'
 
     def __eq__(self, other):
-        return self.tokens == other.tokens and self.metadata == other.metadata
+        return super(TokenList, self).__eq__(other) \
+            and (not hasattr(other, 'metadata') or self.metadata == other.metadata)
+
+    def __ne__(self, other):
+        return not self == other
+
+    def clear(self):
+        self[:] = []  # Supported in Python 2 and 3, unlike clear()
+        self.metadata = None
+
+    def copy(self):
+        tokens_copy = self[:]  # Supported in Python 2 and 3, unlike copy()
+        return TokenList(tokens_copy, self.metadata)
+
+    def extend(self, iterable):
+        super(TokenList, self).extend(iterable)
+        if hasattr(iterable, 'metadata'):
+            if hasattr(self.metadata, '__add__') and hasattr(iterable.metadata, '__add__'):
+                self.metadata += iterable.metadata
+            elif type(self.metadata) is dict and type(iterable.metadata) is dict:
+                # noinspection PyUnresolvedReferences
+                self.metadata.update(iterable.metadata)
+            else:
+                self.metadata = [self.metadata, iterable.metadata]
+
+    @property
+    def tokens(self):
+        return self
+
+    @tokens.setter
+    def tokens(self, value):
+        self[:] = value  # Supported in Python 2 and 3, unlike clear()
 
     def serialize(self):
         return serialize(self)
@@ -39,6 +67,7 @@ class TokenList(object):
         root.set_metadata(self.metadata)
         return root
 
+
 class TokenTree(object):
     token = None
     children = None


Test patch:
diff --git a/tests/test_models.py b/tests/test_models.py
index 02d377a..0e5a182 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -26,6 +26,55 @@ class TestTokenList(unittest.TestCase):
         tokenlist2.metadata = metadata
         self.assertEqual(tokenlist1, tokenlist2)
 
+    def test_len(self):
+        tokenlist = TokenList([{"id": 1}, {"id": 2}, {"id": 3}])
+        self.assertEqual(3, len(tokenlist))
+
+    def test_clear(self):
+        tokenlist = TokenList([{"id": 1}, {"id": 2}, {"id": 3}], {"meta": "data"})
+        tokenlist.clear()
+        self.assertEqual(len(tokenlist.tokens), 0)
+        self.assertEqual(tokenlist.metadata, None)
+
+    def test_copy(self):
+        tokenlist1 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}], {"meta": "data"})
+        tokenlist2 = tokenlist1.copy()
+        self.assertIsNot(tokenlist1, tokenlist2)
+        self.assertEqual(tokenlist1, tokenlist2)
+
+    def test_extend(self):
+        tokenlist1 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}])
+        tokenlist2 = [{"id": 4}, {"id": 5}, {"id": 6}]
+        tokenlist1.extend(tokenlist2)
+        tokenlist3 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}, {"id": 4}, {"id": 5}, {"id": 6}])
+        self.assertEqual(tokenlist1, tokenlist3)
+
+        tokenlist4 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}], {"meta1": "data1"})
+        tokenlist5 = TokenList([{"id": 4}, {"id": 5}, {"id": 6}], {"meta2": "data2"})
+        tokenlist4.extend(tokenlist5)
+        tokenlist6 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}, {"id": 4}, {"id": 5}, {"id": 6}],
+                               {"meta1": "data1", "meta2": "data2"})
+        self.assertEqual(tokenlist4, tokenlist6)
+
+        tokenlist7 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}], "abc")
+        tokenlist8 = TokenList([{"id": 4}, {"id": 5}, {"id": 6}], "de")
+        tokenlist7.extend(tokenlist8)
+        tokenlist9 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}, {"id": 4}, {"id": 5}, {"id": 6}], "abcde")
+        self.assertEqual(tokenlist7, tokenlist9)
+
+        tokenlist7 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}], "abc")
+        tokenlist8 = TokenList([{"id": 4}, {"id": 5}, {"id": 6}], {"meta2": "data2"})
+        tokenlist7.extend(tokenlist8)
+        tokenlist9 = TokenList([{"id": 1}, {"id": 2}, {"id": 3}, {"id": 4}, {"id": 5}, {"id": 6}],
+                               ["abc", {"meta2": "data2"}])
+        self.assertEqual(tokenlist7, tokenlist9)
+
+    def test_tokens(self):
+        tokenlist = TokenList([{"id": 1}, {"id": 2}, {"id": 3}])
+        self.assertEqual(tokenlist.tokens, [{"id": 1}, {"id": 2}, {"id": 3}])
+        tokenlist.tokens = [{"id": 4}, {"id": 5}]
+        self.assertEqual(tokenlist.tokens, [{"id": 4}, {"id": 5}])
+
     def test_to_tree(self):
         tokenlist = TokenList([
             OrderedDict([("id", 2), ("form", "dog"), ("head", 0)]),
@@ -40,11 +89,13 @@ class TestTokenList(unittest.TestCase):
         )
         self.assertEqual(tokenlist.to_tree(), tree)
 
+
 class TestSerialize(unittest.TestCase):
     def test_serialize_on_tokenlist(self):
         tokenlist = TokenList([{"id": 1}])
         self.assertEqual(tokenlist.serialize(), serialize(tokenlist))
 
+
 class TestTokenTree(unittest.TestCase):
     def test_eq(self):
         metadata = {"meta": "data"}
@@ -67,6 +118,7 @@ class TestTokenTree(unittest.TestCase):
         tree.set_metadata(metadata)
         self.assertEqual(tree.metadata, metadata)
 
+
 class TestSerializeTree(unittest.TestCase):
     def test_missing_id(self):
         tree = TokenTree(token={"form": "hej"}, children=[])
@@ -105,6 +157,7 @@ class TestSerializeTree(unittest.TestCase):
             """)
         )
 
+
 class TestPrintTree(unittest.TestCase):
     def test_print_empty_list(self):
         tree = TokenTree(None, [])