-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fix for issue #1009 #1018
Fix for issue #1009 #1018
Changes from 5 commits
4d61547
b621f10
0af3fd0
56deea3
8b17640
8d6774b
2257343
8fb22df
edc7ea8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,7 +80,6 @@ def smart_open(fname, mode='rb'): | |
RE_HTML_ENTITY = re.compile(r'&(#?)([xX]?)(\w{1,8});', re.UNICODE) | ||
|
||
|
||
|
||
def synchronous(tlockname): | ||
""" | ||
A decorator to place an instance-based lock around a method. | ||
|
@@ -1005,15 +1004,13 @@ def pyro_daemon(name, obj, random_suffix=False, ip=None, port=None, ns_conf={}): | |
|
||
def has_pattern(): | ||
""" | ||
Function to check if there is installed pattern library | ||
Function which returns a flag indicating whether pattern is installed or not | ||
""" | ||
pattern = False | ||
try: | ||
from pattern.en import parse | ||
pattern = True | ||
except ImportError: | ||
warnings.warn("Pattern library is not installed, lemmatization won't be available.") | ||
return pattern | ||
import pattern | ||
return True | ||
except: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method is just for testing if pattern is present or not, later to be called in wikicorpus method. Will adding ImportError make any difference? Sorry I am not very good with exception handling. We hardly use them in our academic setting. |
||
return False | ||
|
||
|
||
def lemmatize(content, allowed_tags=re.compile('(NN|VB|JJ|RB)'), light=False, | ||
|
@@ -1037,10 +1034,10 @@ def lemmatize(content, allowed_tags=re.compile('(NN|VB|JJ|RB)'), light=False, | |
['rank/NN', 'study/VB', 'hard/RB'] | ||
|
||
""" | ||
if not has_pattern(): | ||
raise ImportError("Pattern library is not installed. Pattern library is needed in order \ | ||
to use lemmatize function") | ||
from pattern.en import parse | ||
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please only check import in one place There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Please review. |
||
from pattern.en import parse | ||
except: | ||
raise ImportError("Pattern library is not installed. Pattern library is needed in order to use lemmatize function") | ||
|
||
if light: | ||
import warnings | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep
from pattern.en import parse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will change it. Are there any changes required in the exception handling part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have fixed it. During testing there was some bug in Travis's python2.7 and python3.3 initialization. Can you please check that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests seems to be passing now. Can it be merged ?