Add verification when summarize_corpus returns null. Fix #1531.#1570
Conversation
| # If couldn't get important docs, the algorithm ends. | ||
| if not most_important_docs: | ||
| logger.warning("Couldn't get relevant sentences.") | ||
| return |
There was a problem hiding this comment.
Raising an exception better? I'm not sure whether this is an error state, or just a warning.
Many people don't have logging enabled, and the docstring implies the result of this function is a string (not None).
There was a problem hiding this comment.
I don't think this is an error. Perhaps it makes even more sense to actually return the entire text as the summary wasn't possible, but this will break compatibility with the old behavior.
Regarding the docstring, the method returns a string or a list if the split parameter was set to true, so perhaps the best thing to do is:
if not most_important_docs:
logger.warning("Couldn't get relevant sentences.")
return [] if split else ""
There was a problem hiding this comment.
That looks like a good solution 👍
Except we probably want to work with unicode in general ("" => u"", if the rest of the code uses proper unicode too).
CC @menshikh-iv
- Returns empty list on border case of summarize_corpus. - Returns empty string or empty list on border case of summarize. - Fixed test accordingly. - Removed some test code repetition.
menshikh-iv
changed the title
|
Thank you @fbarrios |
3 participants
No description provided.