You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "mawiesne (via GitHub)" <gi...@apache.org> on 2024/12/19 15:44:53 UTC

[PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

mawiesne opened a new pull request, #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200

   Thank you for contributing to Apache OpenNLP.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
   
   - [x] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [x] Have you ensured that the full suite of tests is executed via `mvn clean install` at the root opennlp-sandbox folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](https://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp-sandbox folder?
   - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp-sandbox folder?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions for build issues and submit an update to your PR as soon as possible.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892565760


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WordPOS.java:
##########
@@ -108,7 +115,13 @@ public List<Synset> getSynsets() {
     return Collections.emptyList();
   }
 
-  // uses Stemming to check if two words are equivalent
+  /**
+   * Applies stemming to check whether {@code wordToCompare} is equivalent
+   * to the current {@link WordPOS} instance.
+   *
+   * @param wordToCompare The {@link WordPOS} instance to compare against.
+   */
+  // TODO check the performance of the current implementation (!)

Review Comment:
   Does this deserve a Jira?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892667821


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WTDIMS.java:
##########
@@ -123,31 +123,31 @@ public String[] getSentence() {
     return sentence;
   }
 
-  public void setSentence(String[] sentence) {
+  private void setSentence(String[] sentence) {

Review Comment:
   Which class?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892551223


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/Disambiguator.java:
##########
@@ -24,21 +24,29 @@
 import java.util.List;
 
 /**
- * A word sense disambiguator that determines which sense of a word is meant in
- * a particular context. It is a classification task, where the classes are the
- * different senses of the ambiguous word. Disambiguation can be achieved in
- * either supervised or un-supervised approaches. A {@link Disambiguator} returns
- * a sense ID.
+ * Describes a word sense disambiguator that determines which sense of a word is
+ * meant in a particular context.
+ * It is a classification task, where the classes are the different senses of
+ * the ambiguous word. Disambiguation can be achieved in either supervised or
+ * un-supervised approaches. A {@link Disambiguator} returns a sense ID.
  * <p>
  * <b>How it works:</b><br/>
- * Just supply the context as an array of tokens and the
- * index of the target word to the disambiguate method.
+ * Just supply the {@code context} as an array of tokens and the index of the
+ * {@code target word} to the disambiguate method.
  * <p>
- * Otherwise, for multiple words, you can set a word span instead of simply one
- * index. For the moment the source of sense definitions is from WordNet.
+ * Otherwise, for multiple words, you can set a word {@link Span} instead of
+ * a single target index.
  */
 public interface Disambiguator {
 
+  /**
+   * Conducts disambiguation for a {@link WSDSample} context.
+   *
+   * @param sample    The {@link WSDSample} containing the word and POS tags to use.

Review Comment:
   null allowed here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1893846337


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/FeaturesExtractor.java:
##########
@@ -155,31 +158,43 @@ public List<String> extractTrainingSurroundingWords(List<WTDIMS> trainingData) {
   }
 
   /**
-   * This method generates the different set of features related to the IMS
-   * approach and store them in the corresponding attributes of the {@link WTDIMS}.
+   * Generates the different set of features related to the IMS
+   * approach and puts them in the corresponding attributes of
+   * the {@link WTDIMS word to disambiguate} object.
    * 
    * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param windowSize The parameter required to generate the features qualified of
    *                   "PoS of Surrounding Words".
    * @param ngram The parameter required to generate the features qualified of
    *              "Local Collocations".
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void extractIMSFeatures(WTDIMS wtd, int windowSize, int ngram) {
+    if (wtd == null) {
+      throw new IllegalArgumentException("WTD must not be null");
+    }
     wtd.setPosOfSurroundingWords(extractPosOfSurroundingWords(wtd, windowSize));
     wtd.setSurroundingWords(extractSurroundingWords(wtd));
     wtd.setLocalCollocations(extractLocalCollocations(wtd, ngram));
   }
 
   /**
-   * This generates the context of IMS. It supposes that the features have
-   * already been extracted and stored in the {@link WTDIMS} object, therefore it
-   * doesn't require any parameters.
+   * Generates the context for the {@link WTDIMS word to disambiguate}.
+   *
+   * @implNote It is assumed that the features have already been extracted and
+   * wrapped in the {@link WTDIMS word to disambiguate}.
+   * Therefore, it doesn't require any parameters.
    *
-   * @param wtd The {@link WTDIMS wtd to disambiguate}.
+   * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param listSurrWords The full list of surrounding words of the training data.
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void serializeIMSFeatures(WTDIMS wtd, List<String> listSurrWords) {
-
+    if (wtd == null) {

Review Comment:
   @rzo1 FYI: https://issues.apache.org/jira/browse/OPENNLP-1676



##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/Disambiguator.java:
##########
@@ -24,21 +24,29 @@
 import java.util.List;
 
 /**
- * A word sense disambiguator that determines which sense of a word is meant in
- * a particular context. It is a classification task, where the classes are the
- * different senses of the ambiguous word. Disambiguation can be achieved in
- * either supervised or un-supervised approaches. A {@link Disambiguator} returns
- * a sense ID.
+ * Describes a word sense disambiguator that determines which sense of a word is
+ * meant in a particular context.
+ * It is a classification task, where the classes are the different senses of
+ * the ambiguous word. Disambiguation can be achieved in either supervised or
+ * un-supervised approaches. A {@link Disambiguator} returns a sense ID.
  * <p>
  * <b>How it works:</b><br/>
- * Just supply the context as an array of tokens and the
- * index of the target word to the disambiguate method.
+ * Just supply the {@code context} as an array of tokens and the index of the
+ * {@code target word} to the disambiguate method.
  * <p>
- * Otherwise, for multiple words, you can set a word span instead of simply one
- * index. For the moment the source of sense definitions is from WordNet.
+ * Otherwise, for multiple words, you can set a word {@link Span} instead of
+ * a single target index.
  */
 public interface Disambiguator {
 
+  /**
+   * Conducts disambiguation for a {@link WSDSample} context.
+   *
+   * @param sample    The {@link WSDSample} containing the word and POS tags to use.

Review Comment:
   @rzo1 FYI: https://issues.apache.org/jira/browse/OPENNLP-1676



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892559517


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/FeaturesExtractor.java:
##########
@@ -155,31 +158,43 @@ public List<String> extractTrainingSurroundingWords(List<WTDIMS> trainingData) {
   }
 
   /**
-   * This method generates the different set of features related to the IMS
-   * approach and store them in the corresponding attributes of the {@link WTDIMS}.
+   * Generates the different set of features related to the IMS
+   * approach and puts them in the corresponding attributes of
+   * the {@link WTDIMS word to disambiguate} object.
    * 
    * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param windowSize The parameter required to generate the features qualified of
    *                   "PoS of Surrounding Words".
    * @param ngram The parameter required to generate the features qualified of
    *              "Local Collocations".
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void extractIMSFeatures(WTDIMS wtd, int windowSize, int ngram) {
+    if (wtd == null) {
+      throw new IllegalArgumentException("WTD must not be null");
+    }
     wtd.setPosOfSurroundingWords(extractPosOfSurroundingWords(wtd, windowSize));
     wtd.setSurroundingWords(extractSurroundingWords(wtd));
     wtd.setLocalCollocations(extractLocalCollocations(wtd, ngram));
   }
 
   /**
-   * This generates the context of IMS. It supposes that the features have
-   * already been extracted and stored in the {@link WTDIMS} object, therefore it
-   * doesn't require any parameters.
+   * Generates the context for the {@link WTDIMS word to disambiguate}.
+   *
+   * @implNote It is assumed that the features have already been extracted and
+   * wrapped in the {@link WTDIMS word to disambiguate}.
+   * Therefore, it doesn't require any parameters.
    *
-   * @param wtd The {@link WTDIMS wtd to disambiguate}.
+   * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param listSurrWords The full list of surrounding words of the training data.
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void serializeIMSFeatures(WTDIMS wtd, List<String> listSurrWords) {
-
+    if (wtd == null) {

Review Comment:
   Shouldn'T we check for listSurrWords == null too? Otherwise there is a NPE in line 204 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892564469


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WTDIMS.java:
##########
@@ -123,31 +123,31 @@ public String[] getSentence() {
     return sentence;
   }
 
-  public void setSentence(String[] sentence) {
+  private void setSentence(String[] sentence) {

Review Comment:
   IS this class a candidate for a record?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892669148


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/FeaturesExtractor.java:
##########
@@ -155,31 +158,43 @@ public List<String> extractTrainingSurroundingWords(List<WTDIMS> trainingData) {
   }
 
   /**
-   * This method generates the different set of features related to the IMS
-   * approach and store them in the corresponding attributes of the {@link WTDIMS}.
+   * Generates the different set of features related to the IMS
+   * approach and puts them in the corresponding attributes of
+   * the {@link WTDIMS word to disambiguate} object.
    * 
    * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param windowSize The parameter required to generate the features qualified of
    *                   "PoS of Surrounding Words".
    * @param ngram The parameter required to generate the features qualified of
    *              "Local Collocations".
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void extractIMSFeatures(WTDIMS wtd, int windowSize, int ngram) {
+    if (wtd == null) {
+      throw new IllegalArgumentException("WTD must not be null");
+    }
     wtd.setPosOfSurroundingWords(extractPosOfSurroundingWords(wtd, windowSize));
     wtd.setSurroundingWords(extractSurroundingWords(wtd));
     wtd.setLocalCollocations(extractLocalCollocations(wtd, ngram));
   }
 
   /**
-   * This generates the context of IMS. It supposes that the features have
-   * already been extracted and stored in the {@link WTDIMS} object, therefore it
-   * doesn't require any parameters.
+   * Generates the context for the {@link WTDIMS word to disambiguate}.
+   *
+   * @implNote It is assumed that the features have already been extracted and
+   * wrapped in the {@link WTDIMS word to disambiguate}.
+   * Therefore, it doesn't require any parameters.
    *
-   * @param wtd The {@link WTDIMS wtd to disambiguate}.
+   * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param listSurrWords The full list of surrounding words of the training data.
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void serializeIMSFeatures(WTDIMS wtd, List<String> listSurrWords) {
-
+    if (wtd == null) {

Review Comment:
   Yes, separate follow-up Jira for parameter checking / validation, pls.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892667295


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WordPOS.java:
##########
@@ -108,7 +115,13 @@ public List<Synset> getSynsets() {
     return Collections.emptyList();
   }
 
-  // uses Stemming to check if two words are equivalent
+  /**
+   * Applies stemming to check whether {@code wordToCompare} is equivalent
+   * to the current {@link WordPOS} instance.
+   *
+   * @param wordToCompare The {@link WordPOS} instance to compare against.
+   */
+  // TODO check the performance of the current implementation (!)

Review Comment:
   @rzo1 Yes, it is related to a very slow execution of _MFSEvaluatorIT_. Should be investigated / improved.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892559517


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/FeaturesExtractor.java:
##########
@@ -155,31 +158,43 @@ public List<String> extractTrainingSurroundingWords(List<WTDIMS> trainingData) {
   }
 
   /**
-   * This method generates the different set of features related to the IMS
-   * approach and store them in the corresponding attributes of the {@link WTDIMS}.
+   * Generates the different set of features related to the IMS
+   * approach and puts them in the corresponding attributes of
+   * the {@link WTDIMS word to disambiguate} object.
    * 
    * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param windowSize The parameter required to generate the features qualified of
    *                   "PoS of Surrounding Words".
    * @param ngram The parameter required to generate the features qualified of
    *              "Local Collocations".
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void extractIMSFeatures(WTDIMS wtd, int windowSize, int ngram) {
+    if (wtd == null) {
+      throw new IllegalArgumentException("WTD must not be null");
+    }
     wtd.setPosOfSurroundingWords(extractPosOfSurroundingWords(wtd, windowSize));
     wtd.setSurroundingWords(extractSurroundingWords(wtd));
     wtd.setLocalCollocations(extractLocalCollocations(wtd, ngram));
   }
 
   /**
-   * This generates the context of IMS. It supposes that the features have
-   * already been extracted and stored in the {@link WTDIMS} object, therefore it
-   * doesn't require any parameters.
+   * Generates the context for the {@link WTDIMS word to disambiguate}.
+   *
+   * @implNote It is assumed that the features have already been extracted and
+   * wrapped in the {@link WTDIMS word to disambiguate}.
+   * Therefore, it doesn't require any parameters.
    *
-   * @param wtd The {@link WTDIMS wtd to disambiguate}.
+   * @param wtd The {@link WTDIMS word to disambiguate}.
    * @param listSurrWords The full list of surrounding words of the training data.
+   *
+   * @throws IllegalArgumentException Thrown if parameters were invalid.
    */
   public void serializeIMSFeatures(WTDIMS wtd, List<String> listSurrWords) {
-
+    if (wtd == null) {

Review Comment:
   Shouldn'T we check for listSurrWords == null too? Otherwise there is a NPE in 218 ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892807274


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WTDIMS.java:
##########
@@ -123,31 +123,31 @@ public String[] getSentence() {
     return sentence;
   }
 
-  public void setSentence(String[] sentence) {
+  private void setSentence(String[] sentence) {

Review Comment:
   potentially yes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892667295


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WordPOS.java:
##########
@@ -108,7 +115,13 @@ public List<Synset> getSynsets() {
     return Collections.emptyList();
   }
 
-  // uses Stemming to check if two words are equivalent
+  /**
+   * Applies stemming to check whether {@code wordToCompare} is equivalent
+   * to the current {@link WordPOS} instance.
+   *
+   * @param wordToCompare The {@link WordPOS} instance to compare against.
+   */
+  // TODO check the performance of the current implementation (!)

Review Comment:
   @rzo1 Yes, it is related to a very slow execution of _LeskEvaluatorIT_. Should be investigated / improved.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne merged PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892551223


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/Disambiguator.java:
##########
@@ -24,21 +24,29 @@
 import java.util.List;
 
 /**
- * A word sense disambiguator that determines which sense of a word is meant in
- * a particular context. It is a classification task, where the classes are the
- * different senses of the ambiguous word. Disambiguation can be achieved in
- * either supervised or un-supervised approaches. A {@link Disambiguator} returns
- * a sense ID.
+ * Describes a word sense disambiguator that determines which sense of a word is
+ * meant in a particular context.
+ * It is a classification task, where the classes are the different senses of
+ * the ambiguous word. Disambiguation can be achieved in either supervised or
+ * un-supervised approaches. A {@link Disambiguator} returns a sense ID.
  * <p>
  * <b>How it works:</b><br/>
- * Just supply the context as an array of tokens and the
- * index of the target word to the disambiguate method.
+ * Just supply the {@code context} as an array of tokens and the index of the
+ * {@code target word} to the disambiguate method.
  * <p>
- * Otherwise, for multiple words, you can set a word span instead of simply one
- * index. For the moment the source of sense definitions is from WordNet.
+ * Otherwise, for multiple words, you can set a word {@link Span} instead of
+ * a single target index.
  */
 public interface Disambiguator {
 
+  /**
+   * Conducts disambiguation for a {@link WSDSample} context.
+   *
+   * @param sample    The {@link WSDSample} containing the word and POS tags to use.

Review Comment:
   null allowed here? -> ah unclear for most of the methods in that interface. So not an issue for now (follow up)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OPENNLP-789: Extend JavaDoc for WSD component (opennlp-sandbox)

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #200:
URL: https://github.com/apache/opennlp-sandbox/pull/200#discussion_r1892671246


##########
opennlp-wsd/src/main/java/opennlp/tools/disambiguator/WTDIMS.java:
##########
@@ -123,31 +123,31 @@ public String[] getSentence() {
     return sentence;
   }
 
-  public void setSentence(String[] sentence) {
+  private void setSentence(String[] sentence) {

Review Comment:
   wtdims



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org