Use abstract classes #1639
Use abstract classes #1639
Conversation
| def __str__(self): | ||
| raise NotImplementedError() | ||
| pass |
There was a problem hiding this comment.
The pass here is inconsistent with the rest of the @abstractmethod. Is it necessary?
There was a problem hiding this comment.
Ah my mistake there's no docstring for this, that's why =)
nltk/metrics/association.py
Outdated
| def _contingency(*marginals): | ||
| """Calculates values of a contingency table from marginal values.""" | ||
| raise NotImplementedError("The contingency table is not available" | ||
| "in the general ngram case") |
There was a problem hiding this comment.
For these cases I think the error message is important. Is there a way to use @abstractmethod and still raise the error message?
There was a problem hiding this comment.
good idea, added it back!
nltk/metrics/association.py
Outdated
| def _marginals(*contingency): | ||
| """Calculates values of contingency table marginals from its values.""" | ||
| raise NotImplementedError("The contingency table is not available" | ||
| "in the general ngram case") |
There was a problem hiding this comment.
Same as above.
For these cases I think the error message is important. Is there a way to use @abstractmethod and still raise the error message?
There was a problem hiding this comment.
good idea, added it back!
|
@alvations thanks for the feedback - reflected |
|
Rebased and updated. |
|
@alvations @stevenbird - does this look better? |
|
@naoyak – yes, looking good! |
Ref: #1130
Cherry-picking some commits from #1562.