-
Notifications
You must be signed in to change notification settings - Fork 4
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
Assert that locations are not empty #5
Assert that locations are not empty #5
Conversation
6caee37
to
4f28e09
Compare
Before that an empty locations list would raise some weird pyahocorasick error about a value not being an automaton
4f28e09
to
ed31ce2
Compare
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.
Thanks for your contribution! There's a small change that I'd like to see. In addition, please add a corresponding test case to the TestNameExtractor
class.
geoextract/__init__.py
Outdated
|
||
if not list(self._automaton.items()): | ||
raise ValueError("You must provide locations for extraction") | ||
|
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'm fine with adding this check, but it should be done at the end of NameExtractor.setup
instead. At that point, no further words will be added to the automaton.
Thanks for your contribution, @konstin! |
In hindsight, I think the better approach is to simply return no matches from I've made this change in 0d184e2. |
Ah great, that makes it even easier to use! |
Before that an empty locations list would raise some weird pyahocorasick error about a value not being an automaton