-
-
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 Issue #805: Added check in summarize_corpus #885
Conversation
anmolgulati
commented
Sep 25, 2016
•
edited
Loading
edited
- Added check in sumarize_corpus to reject summarizing if number of nodes in graph is less than 3
- This resolves Issue Error while summarizing text #805
Thanks for taking this up. Please add a test for the warning. |
@@ -158,6 +158,11 @@ def summarize_corpus(corpus, ratio=0.2): | |||
_set_graph_edge_weights(graph) | |||
_remove_unreachable_nodes(graph) | |||
|
|||
# If the number of nodes < 3, the function ends. |
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 add a better comment explaining original error
Looks good. Please add a note in the CHANGELOG for 0.13.3 release and will merge once the tests pass. |
I've made the changes now. |
Hi @anmol01gulati , it is some travis weirdness with |
Added blank line to fix issue with travis CI
Great. Thanks for the pr! |
…words (piskvorky#885) * Added check in summarize_corpus to fix bug in summarizer * Fix piskvorky#805: Added check in summarizing text * Added test for checking low number of distinct words in text * Text split method changed to allow running in Python 3.3 and above. * Change to fix test in python versions 3.3 and higher * Added blank line test_wikicorpus.py file Added blank line to fix issue with travis CI
# Cannot calculate eigenvectors if number of unique words in text < 3. Warns user to add more text. The function ends. | ||
if len(graph.nodes()) < 3: | ||
logger.warning("Please add more sentences to the text. The number of reachable nodes is below 3") | ||
return |
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.
This doesn't match the function signature (Returns a list
). Either raise an exception, or return an empty list -- this is just nasty unexpected gotcha for users.