allenai / allennlp-semparse

A framework for building semantic parsers (including neural module networks) with AllenNLP, built by the authors of AllenNLP
Apache License 2.0
107 stars 24 forks source link

KnowledgeGraphField.empty_field is broken #14

Open OhadRubin opened 5 years ago

OhadRubin commented 5 years ago

instances of KnowledgeGraphField are throwing AttributeError when using the method empty_field.

Steps to repreduce:

from allennlp.tests.data.fields.knowledge_graph_field_test import KnowledgeGraphFieldTest a = KnowledgeGraphFieldTest() a.setUp() a.field.empty_field()

Output:

AttributeError Traceback (most recent call last)

in 35 a = KnowledgeGraphFieldTest() 36 a.setUp() ---> 37 a.field.empty_field() ~/.local/lib/python3.6/site-packages/allennlp/data/fields/knowledge_graph_field.py in empty_field(self) 257 @overrides 258 def empty_field(self) -> 'KnowledgeGraphField': --> 259 return KnowledgeGraphField(KnowledgeGraph(set(), {}), [], self._token_indexers) 260 261 @overrides ~/.local/lib/python3.6/site-packages/allennlp/data/fields/knowledge_graph_field.py in __init__(self, knowledge_graph, utterance_tokens, token_indexers, tokenizer, feature_extractors, entity_tokens, linking_features, include_in_vocab, max_table_tokens) 104 # hand-written features, like with a CNN, we can cut down our data processing time by a 105 # factor of 2. --> 106 self.entity_texts = tokenizer.batch_tokenize(entity_texts) 107 else: 108 self.entity_texts = entity_tokens AttributeError: 'NoneType' object has no attribute 'batch_tokenize'
DeNeutoy commented 5 years ago

Hi! Looks like this is a small bug, which you can see here:

https://github.com/allenai/allennlp/blob/master/allennlp/data/fields/knowledge_graph_field.py#L90

where we don't catch whether the tokenizer is None (it should be either non-optional or set to some default option). We'd appreciate a PR!