allenai / allennlp

An open-source NLP research library, built on PyTorch.
http://www.allennlp.org
Apache License 2.0
11.73k stars 2.24k forks source link

Cannot import DomainLanguage and a bunch of stuff: ImportError: cannot import name 'TableQuestionContext' from 'allennlp.semparse.contexts' #2935

Closed nilesh-c closed 5 years ago

nilesh-c commented 5 years ago

Describe the bug Trying to import DomainLanguage gives this error:

In [1]: from allennlp.semparse.domain_languages.domain_language import DomainLanguage                                                                                                                                      
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-4847c7b12c69> in <module>
----> 1 from allennlp.semparse.domain_languages.domain_language import DomainLanguage

~/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/__init__.py in <module>
     29 # dependency issues.  If you want to import semparse stuff from the data code, just use a more
     30 # complete path, like `from allennlp.semparse.worlds import WikiTablesWorld`.
---> 31 from allennlp.data.tokenizers import Token as _
     32 from allennlp.semparse.domain_languages.domain_language import (DomainLanguage, ParsingError,
     33                                                                 ExecutionError,

...

ImportError: cannot import name 'TableQuestionContext' from 'allennlp.semparse.contexts' (/data/nilesh/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/contexts/__init__.py)

But trying it again, works:

In [2]: from allennlp.semparse.domain_languages.domain_language import DomainLanguage                                                                                                                                      

In [3]: DomainLanguage                                                                                                                                                                                                     
Out[3]: allennlp.semparse.domain_languages.domain_language.DomainLanguage

Looks like some kind of circular dependencies? This is not happening on my Mac OS X, only on Ubuntu.

To Reproduce Steps to reproduce the behavior

  1. Go to ipython
  2. Type from allennlp.semparse.domain_languages.domain_language import DomainLanguage

System (please complete the following information):

matt-gardner commented 5 years ago

I'm able to reproduce this on my mac. I'll see if I can dig into it, but if you can figure out a fix, it might be faster, and it'd be much appreciated.

nilesh-c commented 5 years ago

@matt-gardner makes sense. I have v0.8.3 on my mac and that works fine. Something broke between the versions. I'll try taking a look too.

matt-gardner commented 5 years ago

@pdasigi had a recent PR that modified the TableQuestionContext; it's probably worth checking if that PR introduced the problem.

nilesh-c commented 5 years ago

Yup, this is the commit that introduced the bug: https://github.com/allenai/allennlp/commit/f8d2ec5b8fa22ff051ad83623327c1bccae653b9#diff-da50db5bd82ad4c8f0df78e3245b9d31

Checked out the one before that and importing works.

nilesh-c commented 5 years ago

So the problem occurs because of this cycle:

~/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/contexts/__init__.py in <module>
      1 from allennlp.semparse.contexts.table_question_knowledge_graph import TableQuestionKnowledgeGraph
      2 from allennlp.semparse.contexts.atis_sql_table_context import AtisSqlTableContext
----> 3 from allennlp.semparse.contexts.table_question_context import TableQuestionContext

~/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/contexts/table_question_context.py in <module>
      6 from unidecode import unidecode
      7 from allennlp.data.tokenizers import Token
----> 8 from allennlp.semparse.domain_languages.common import Date
      9 from allennlp.semparse.contexts.knowledge_graph import KnowledgeGraph
     10 

~/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/domain_languages/__init__.py in <module>
      4 from allennlp.semparse.domain_languages.nlvr_language import NlvrLanguage
      5 from allennlp.semparse.domain_languages.quarel_language import QuaRelLanguage
----> 6 from allennlp.semparse.domain_languages.wikitables_language import WikiTablesLanguage

~/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/domain_languages/wikitables_language.py in <module>
     11                                                                 PredicateType, predicate)
     12 from allennlp.semparse.contexts.table_question_knowledge_graph import MONTH_NUMBERS
---> 13 from allennlp.semparse.contexts import TableQuestionContext
     14 from allennlp.semparse.contexts.table_question_context import CellValueType
     15 from allennlp.semparse.domain_languages.common import Date

ImportError: cannot import name 'TableQuestionContext' from 'allennlp.semparse.contexts' (/data/nilesh/anaconda3/envs/allennlp/lib/python3.7/site-packages/allennlp/semparse/contexts/__init__.py)

table_question_context.py imports Date (introduced in the above commit), and this brings in the domain_languages namespace, which imports WikiTablesLanguage which imports TableQuestionContext and boom.

Removing from allennlp.semparse.domain_languages.wikitables_language import WikiTablesLanguage from allennlp.semparse.domain_languages.__init__.py fixes the problem. Is there a better way?

nilesh-c commented 5 years ago

If context stuff is used by domain_languages stuff, we probably can't have it both ways. So either domain_languages.common.Date has to be moved to a new package semparse.common, or we can't have table_question_context.py using domain-language-specific stuff like Date (which sounds cleaner).

matt-gardner commented 5 years ago

Thanks for digging into this! I think the best solution here is to move Date to semparse.common. Care to submit a PR?

nilesh-c commented 5 years ago

@matt-gardner sure, no problem. Had to move the exception classes from domain_language too because of a dependency. Submitted a PR here: https://github.com/allenai/allennlp/pull/2938