[
https://issues.apache.org/jira/browse/OPENNLP-1594?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17865657#comment-17865657
]
ASF GitHub Bot commented on OPENNLP-1594:
-----------------------------------------
kinow commented on code in PR #158:
URL: https://github.com/apache/opennlp-sandbox/pull/158#discussion_r1676778913
##########
summarizer/src/main/java/opennlp/summarization/Sentence.java:
##########
@@ -113,38 +123,21 @@ public List<Sentence> getLinks() {
return this.links;
}
- public double getWordWt() {
+ public double getWordWeight() {
return wordWt;
}
- public void setWordWt(double wordWt) {
+ public void setWordWeight(double wordWt) {
this.wordWt = wordWt;
}
public int getWordCnt() {
Review Comment:
Since the `Wt` was de-abbreviated, maybe `cnt` could also become `count`? Or
maybe something for a future change...
##########
summarizer/src/main/java/opennlp/summarization/lexicalchaining/LexChainingKeywordExtractor.java:
##########
@@ -22,20 +22,45 @@
import java.util.List;
/**
- * Uses the lexical chaining algorithm to extract keywords.
+ * Uses the {@link LexicalChain lexical chaining} algorithm to extract
keywords.
+ *
+ * @see LexicalChain
*/
public class LexChainingKeywordExtractor {
- // Simple logic to pull out the keyword based on longest lexical chains..
- public List<String> getKeywords(List<LexicalChain> lexicalChains, int
noOfKeywords) {
- Collections.sort(lexicalChains);
- List<String> ret = new ArrayList<>();
- for (int i = 0; i < Math.min(lexicalChains.size(), noOfKeywords); i++) {
- List<Word> words = lexicalChains.get(i).getWord();
- if (!words.isEmpty() && !ret.contains(words.get(0).getLexicon())) {
- ret.add(words.get(0).getLexicon());
+ /**
+ * Extracts keywords from a list of {@link LexicalChain lexical chains},
limited by {@code noOfKeywords}.
+ *
+ * @param lexicalChains The {@link LexicalChain lexical chains} to process.
Must not be {@code null}.
+ * @param noOfKeywords The upper limit of keywords. Must be greater than
{@code zero}.
+ *
+ * @return The extracted keywords as a list. Guaranteed to be not {@code
null}.
+ *
+ * @throws IllegalArgumentException Thrown if parameters are invalid.
+ * @implNote This operation is based on longest lexical chains.
Review Comment:
Interesting, I never used `@implNote`, but searched about it, and learned
something new today! Thanks!!
##########
summarizer/src/main/java/opennlp/summarization/lexicalchaining/LexicalChainingSummarizer.java:
##########
@@ -44,95 +48,130 @@ public class LexicalChainingSummarizer implements
Summarizer {
private final DocProcessor docProcessor;
private final WordRelationshipDetermination wordRel;
- public LexicalChainingSummarizer(DocProcessor dp, OpenNLPPOSTagger
posTagger) {
- docProcessor = dp;
- tagger = posTagger;
- wordRel = new WordRelationshipDetermination();
+ /**
+ * Instantiates a {@link LexicalChainingSummarizer}.
+ *
+ * @param docProcessor The {@link DocProcessor} to use at runtime. Must not
be {@code null}.
+ * @param languageCode An ISO-language code for obtaining a {@link POSModel}.
+ * Must not be {@code null}.
+ *
+ * @throws IllegalArgumentException Thrown if parameters are invalid.
+ */
+ public LexicalChainingSummarizer(DocProcessor docProcessor, String
languageCode) throws IOException {
+ this(docProcessor, new NounPOSTagger(languageCode));
}
- public LexicalChainingSummarizer(DocProcessor dp, InputStream posModelFile)
throws Exception {
- this(dp, new OpenNLPPOSTagger(dp, posModelFile));
+ /**
+ * Instantiates a {@link LexicalChainingSummarizer}.
+ *
+ * @param docProcessor The {@link DocProcessor} to use at runtime. Must not
be {@code null}.
+ * @param posTagger The {@link NounPOSTagger} to use at runtime. Must not be
{@code null}.
+ *
+ * @throws IllegalArgumentException Thrown if parameters are invalid.
+ */
+ public LexicalChainingSummarizer(DocProcessor docProcessor, NounPOSTagger
posTagger) {
+ if (docProcessor == null) throw new IllegalArgumentException("Parameter
'docProcessor' must not be null!");
+ if (posTagger == null) throw new IllegalArgumentException("Parameter
'posTagger' must not be null!");
+
+ this.docProcessor = docProcessor;
+ tagger = posTagger;
+ wordRel = new WordRelationshipDetermination();
}
- //Build Lexical chains..
- public List<LexicalChain> buildLexicalChains(String article, List<Sentence>
sent) {
- // POS tag article
- Hashtable<String, List<LexicalChain>> chains = new Hashtable<>();
- List<LexicalChain> lc = new ArrayList<>();
- // Build lexical chains
- // For each sentence
- for (Sentence currSent : sent) {
- String taggedSent = tagger.getTaggedString(currSent.getStringVal());
- List<String> nouns = tagger.getWordsOfType(taggedSent, POSTagger.NOUN);
- // For each noun
- for (String noun : nouns) {
- int chainsAddCnt = 0;
- // Loop through each LC
- for (LexicalChain l : lc) {
- try {
- WordRelation rel = wordRel.getRelation(l, noun,
(currSent.getSentId() - l.start) > 7);
- // Is the noun an exact match to one of the current LCs (Strong
relation)
- // Add sentence to chain
- if (rel.relation() == WordRelation.STRONG_RELATION) {
- addToChain(rel.dest(), l, chains, currSent);
- if (currSent.getSentId() - l.last > 10) {
- l.occurrences++;
- l.start = currSent.getSentId();
- }
- chainsAddCnt++;
- } else if (rel.relation() == WordRelation.MED_RELATION) {
- // Add sentence to chain if it is 7 sentences away from start
of chain
- addToChain(rel.dest(), l, chains, currSent);
- chainsAddCnt++;
- //If greater than 7 we will add it but call it a new occurrence
of the lexical chain...
- if (currSent.getSentId() - l.start > 7) {
- l.occurrences++;
- l.start = currSent.getSentId();
- }
- } else if (rel.relation() == WordRelation.WEAK_RELATION) {
- if (currSent.getSentId() - l.start <= 3) {
+ /**
+ * Constructs a list of {@link LexicalChain lexical chains} from specified
sentences.
+ *
+ * @param article TODO unused parameter -> remove it?!
Review Comment:
If not used and there are no other classes (siblings/parents, or just very
similar with exact list of params
> Add stricter tests for Summarizer component
> -------------------------------------------
>
> Key: OPENNLP-1594
> URL: https://issues.apache.org/jira/browse/OPENNLP-1594
> Project: OpenNLP
> Issue Type: Test
> Affects Versions: 2.3.3
> Reporter: Martin Wiesner
> Assignee: Martin Wiesner
> Priority: Major
> Fix For: 2.4.0
>
>
> While OPENNLP-1592 and OPENNLP-1593 raised test coverage for the summarizer
> component, there are still some "dark spots" and probably inaccuracies in the
> implementation of that component. More, stricter tests shall shed light on
> the situation.
> Aims:
> * add further, stricter tests for the summarizer component
> * clarify, at API level, the semantics and constraints of parameters
> * better separate tests so that each test class has a clear responsibility
> for its class under test
> * improve / enhance the JavaDoc further
--
This message was sent by Atlassian Jira
(v8.20.10#820010)