make api more pythonic (#1926)

* first stab at contextual encoder wrappers

* contextual encoders

* remove sru encoder

* pr comments

* replace _ElmoCharacterEncoder with CharacterEncoder

* docs

* sphinx stuff

* address pr comments

* address more PR comments

* make sphinx happy

* iterate

* make parameters required

* this is still wip

* wip

* bidirectional-lm proof of concept

* progress

* revert elmo

* revert elmo test

* revert elmo token embedder

* cnn_highway_encoder -> seq2vec

* remove contextual encoders

* fix docs

* remove print

* address more feedback

* replace none with identity function

* fix docs + checks

* fix tests

* add comments

* add top level imports

* fix imports

* unused import

* progress

* use brendan's dataset reader

* make data interface more pythonic

* remove unused import

* fix pytest + pylint

Author: joelgrus

allennlp/data/fields/index_field.py

@@ -51,3 +51,11 @@ def empty_field(self):
 
     def __str__(self) -> str:
         return f"IndexField with index: {self.sequence_index}."
+
+    def __eq__(self, other) -> bool:
+        # Allow equality checks to ints that are the sequence index
+        if isinstance(other, int):
+            return self.sequence_index == other
+        # Otherwise it has to be the same object
+        else:
+            return id(other) == id(self)

allennlp/data/fields/list_field.py

@@ -1,5 +1,5 @@
 # pylint: disable=no-self-use
-from typing import Dict, List
+from typing import Dict, List, Iterator
 
 from overrides import overrides
 
@@ -31,6 +31,16 @@ def __init__(self, field_list: List[Field]) -> None:
         # Not sure why mypy has a hard time with this type...
         self.field_list: List[Field] = field_list
 
+    # Sequence[Field] methods
+    def __iter__(self) -> Iterator[Field]:
+        return iter(self.field_list)
+
+    def __getitem__(self, idx: int) -> Field:
+        return self.field_list[idx]
+
+    def __len__(self) -> int:
+        return len(self.field_list)
+
     @overrides
     def count_vocab_items(self, counter: Dict[str, Dict[str, int]]):
         for field in self.field_list:

allennlp/data/fields/metadata_field.py

@@ -1,12 +1,12 @@
 # pylint: disable=no-self-use
-from typing import Any, Dict, List
+from typing import Any, Dict, List, Mapping
 
 from overrides import overrides
 
 from allennlp.data.fields.field import DataArray, Field
 
 
-class MetadataField(Field[DataArray]):
+class MetadataField(Field[DataArray], Mapping[str, Any]):
     """
     A ``MetadataField`` is a ``Field`` that does not get converted into tensors.  It just carries
     side information that might be needed later on, for computing some third-party metric, or
@@ -27,6 +27,24 @@ class MetadataField(Field[DataArray]):
     def __init__(self, metadata: Any) -> None:
         self.metadata = metadata
 
+    def __getitem__(self, key: str) -> Any:
+        try:
+            return self.metadata[key]  # type: ignore
+        except TypeError:
+            raise TypeError("your metadata is not a dict")
+
+    def __iter__(self):
+        try:
+            return iter(self.metadata)
+        except TypeError:
+            raise TypeError("your metadata is not iterable")
+
+    def __len__(self):
+        try:
+            return len(self.metadata)
+        except TypeError:
+            raise TypeError("your metadata has no length")
+
     @overrides
     def get_padding_lengths(self) -> Dict[str, int]:
         return {}

allennlp/data/fields/sequence_label_field.py

@@ -1,4 +1,4 @@
-from typing import Dict, List, Union, Set
+from typing import Dict, List, Union, Set, Iterator
 import logging
 import textwrap
 
@@ -75,6 +75,16 @@ def _maybe_warn_for_namespace(self, label_namespace: str) -> None:
                                self._label_namespace)
                 self._already_warned_namespaces.add(label_namespace)
 
+    # Sequence methods
+    def __iter__(self) -> Iterator[Union[str, int]]:
+        return iter(self.labels)
+
+    def __getitem__(self, idx: int) -> Union[str, int]:
+        return self.labels[idx]
+
+    def __len__(self) -> int:
+        return len(self.labels)
+
     @overrides
     def count_vocab_items(self, counter: Dict[str, Dict[str, int]]):
         if self._indexed_labels is None:

allennlp/data/fields/span_field.py

@@ -58,3 +58,9 @@ def empty_field(self):
 
     def __str__(self) -> str:
         return f"SpanField with spans: ({self.span_start}, {self.span_end})."
+
+    def __eq__(self, other) -> bool:
+        if isinstance(other, tuple) and len(other) == 2:
+            return other == (self.span_start, self.span_end)
+        else:
+            return id(self) == id(other)

allennlp/data/fields/text_field.py

@@ -2,7 +2,7 @@
 A ``TextField`` represents a string of text, the kind that you might want to represent with
 standard word vectors, or pass through an LSTM.
 """
-from typing import Dict, List, Optional
+from typing import Dict, List, Optional, Iterator
 import textwrap
 
 from overrides import overrides
@@ -44,6 +44,16 @@ def __init__(self, tokens: List[Token], token_indexers: Dict[str, TokenIndexer])
             raise ConfigurationError("TextFields must be passed Tokens. "
                                      "Found: {} with types {}.".format(tokens, [type(x) for x in tokens]))
 
+    # Sequence[Token] methods
+    def __iter__(self) -> Iterator[Token]:
+        return iter(self.tokens)
+
+    def __getitem__(self, idx: int) -> Token:
+        return self.tokens[idx]
+
+    def __len__(self) -> int:
+        return len(self.tokens)
+
     @overrides
     def count_vocab_items(self, counter: Dict[str, Dict[str, int]]):
         for indexer in self._token_indexers.values():

allennlp/data/instance.py

@@ -1,10 +1,10 @@
-from typing import Dict, MutableMapping
+from typing import Dict, MutableMapping, Mapping
 
 from allennlp.data.fields.field import DataArray, Field
 from allennlp.data.vocabulary import Vocabulary
 
 
-class Instance:
+class Instance(Mapping[str, Field]):
     """
     An ``Instance`` is a collection of :class:`~allennlp.data.fields.field.Field` objects,
     specifying the inputs and outputs to
@@ -26,6 +26,18 @@ def __init__(self, fields: MutableMapping[str, Field]) -> None:
         self.fields = fields
         self.indexed = False
 
+    # Add methods for ``Mapping``.  Note, even though the fields are
+    # mutable, we don't implement ``MutableMapping`` because we want
+    # you to use ``add_field`` and supply a vocabulary.
+    def __getitem__(self, key: str) -> Field:
+        return self.fields[key]
+
+    def __iter__(self):
+        return iter(self.fields)
+
+    def __len__(self) -> int:
+        return len(self.fields)
+
     def add_field(self, field_name: str, field: Field, vocab: Vocabulary = None) -> None:
         """
         Add the field to the existing fields mapping.

allennlp/tests/data/fields/index_field_test.py

@@ -31,3 +31,11 @@ def test_index_field_empty_field_works(self):
 
     def test_printing_doesnt_crash(self):
         print(self.text)
+
+    def test_equality(self):
+        index_field1 = IndexField(4, self.text)
+        index_field2 = IndexField(4, self.text)
+
+        assert index_field1 == 4
+        assert index_field1 == index_field1
+        assert index_field1 != index_field2

allennlp/tests/data/fields/list_field_test.py

@@ -181,3 +181,10 @@ def test_as_tensor_can_handle_multiple_token_indexers_and_empty_fields(self):
     def test_printing_doesnt_crash(self):
         list_field = ListField([self.field1, self.field2])
         print(list_field)
+
+    def test_sequence_methods(self):
+        list_field = ListField([self.field1, self.field2, self.field3])
+
+        assert len(list_field) == 3
+        assert list_field[1] == self.field2
+        assert [f for f in list_field] == [self.field1, self.field2, self.field3]

allennlp/tests/data/fields/metadata_field_test.py

@@ -0,0 +1,34 @@
+# pylint: disable=no-self-use,invalid-name
+import pytest
+
+from allennlp.common.testing.test_case import AllenNlpTestCase
+from allennlp.data.fields import MetadataField
+
+
+class TestMetadataField(AllenNlpTestCase):
+    def test_mapping_works_with_dict(self):
+        field = MetadataField({"a": 1, "b": [0]})
+
+        assert "a" in field
+        assert field["a"] == 1
+        assert len(field) == 2
+
+        keys = {k for k in field}
+        assert keys == {"a", "b"}
+
+        values = [v for v in field.values()]
+        assert len(values) == 2
+        assert 1 in values
+        assert [0] in values
+
+    def test_mapping_raises_with_non_dict(self):
+        field = MetadataField(0)
+
+        with pytest.raises(TypeError):
+            _ = field[0]
+
+        with pytest.raises(TypeError):
+            _ = len(field)
+
+        with pytest.raises(TypeError):
+            _ = [x for x in field]

allennlp/tests/data/fields/sequence_label_field_test.py

@@ -88,3 +88,11 @@ def test_printing_doesnt_crash(self):
         tags = ["B", "I", "O", "O", "O"]
         sequence_label_field = SequenceLabelField(tags, self.text, label_namespace="labels")
         print(sequence_label_field)
+
+    def test_sequence_methods(self):
+        tags = ["B", "I", "O", "O", "O"]
+        sequence_label_field = SequenceLabelField(tags, self.text, label_namespace="labels")
+
+        assert len(sequence_label_field) == 5
+        assert sequence_label_field[1] == "I"
+        assert [label for label in sequence_label_field] == tags

allennlp/tests/data/fields/span_field_test.py

@@ -40,3 +40,11 @@ def test_empty_span_field_works(self):
     def test_printing_doesnt_crash(self):
         span_field = SpanField(2, 3, self.text)
         print(span_field)
+
+    def test_equality(self):
+        span_field1 = SpanField(2, 3, self.text)
+        span_field2 = SpanField(2, 3, self.text)
+
+        assert span_field1 == (2, 3)
+        assert span_field1 == span_field1
+        assert span_field1 != span_field2

allennlp/tests/data/fields/text_field_test.py

@@ -253,3 +253,10 @@ def test_token_embedder_returns_dict(self):
         assert list(tensors['additional_key'].shape) == [3]
         assert list(tensors['words'].shape) == [4]
         assert list(tensors['characters'].shape) == [4, 8]
+
+    def test_sequence_methods(self):
+        field = TextField([Token(t) for t in ["This", "is", "a", "sentence", "."]], {})
+
+        assert len(field) == 5
+        assert field[1].text == "is"
+        assert [token.text for token in field] == ["This", "is", "a", "sentence", "."]

allennlp/tests/data/instance_test.py

@@ -0,0 +1,22 @@
+# pylint: disable=no-self-use,invalid-name
+from allennlp.common.testing import AllenNlpTestCase
+from allennlp.data import Instance
+from allennlp.data.fields import TextField, LabelField
+from allennlp.data.tokenizers import Token
+
+class TestInstance(AllenNlpTestCase):
+    def test_instance_implements_mutable_mapping(self):
+        words_field = TextField([Token("hello")], {})
+        label_field = LabelField(1, skip_indexing=True)
+        instance = Instance({"words": words_field, "labels": label_field})
+
+        assert instance["words"] == words_field
+        assert instance["labels"] == label_field
+        assert len(instance) == 2
+
+        keys = {k for k, v in instance.items()}
+        assert keys == {"words", "labels"}
+
+        values = [v for k, v in instance.items()]
+        assert words_field in values
+        assert label_field in values

allennlp/tests/data/iterators/basic_iterator_test.py

@@ -155,8 +155,8 @@ def test_shuffle(self):
             assert in_order_batches != shuffled_batches
 
             # But not the counts of the instances.
-            in_order_counts = Counter(instance for batch in in_order_batches for instance in batch)
-            shuffled_counts = Counter(instance for batch in shuffled_batches for instance in batch)
+            in_order_counts = Counter(id(instance) for batch in in_order_batches for instance in batch)
+            shuffled_counts = Counter(id(instance) for batch in shuffled_batches for instance in batch)
             assert in_order_counts == shuffled_counts