[ 
https://issues.apache.org/jira/browse/OPENNLP-1169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16294990#comment-16294990
 ] 

ASF GitHub Bot commented on OPENNLP-1169:
-----------------------------------------

tteofili closed pull request #297: OPENNLP-1169 - WVTable fetches WVs by String
URL: https://github.com/apache/opennlp/pull/297
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/DoubleArrayVector.java
 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/DoubleArrayVector.java
index 79d03a3af..e00ab808f 100644
--- 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/DoubleArrayVector.java
+++ 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/DoubleArrayVector.java
@@ -22,7 +22,7 @@
 
 class DoubleArrayVector implements WordVector {
 
-  private double[] vector;
+  private final double[] vector;
 
   DoubleArrayVector(double[] vector) {
     this.vector = vector;
diff --git 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/FloatArrayVector.java
 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/FloatArrayVector.java
index 0ee728764..a0fd13e98 100644
--- 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/FloatArrayVector.java
+++ 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/FloatArrayVector.java
@@ -22,7 +22,7 @@
 
 class FloatArrayVector implements WordVector {
 
-  private float[] vector;
+  private final float[] vector;
 
   FloatArrayVector(float[] vector) {
     this.vector = vector;
diff --git 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/Glove.java 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/Glove.java
index c0c405338..91c2ff1da 100644
--- a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/Glove.java
+++ b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/Glove.java
@@ -43,9 +43,9 @@ private Glove() {
    * <p>
    * Warning: Experimental new feature, see OPENNLP-1144 for details, the API 
might be changed anytime.
    *
-   * @param in
-   * @return
-   * @throws IOException
+   * @param in the input stream for Glove vectors
+   * @return a Glove based wv table
+   * @throws IOException if any error occurs during parsing
    */
   @Experimental
   public static WordVectorTable parse(InputStream in) throws IOException {
diff --git 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/MapWordVectorTable.java
 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/MapWordVectorTable.java
index 83776d802..763a69c56 100644
--- 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/MapWordVectorTable.java
+++ 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/MapWordVectorTable.java
@@ -28,8 +28,8 @@
   }
 
   @Override
-  public WordVector get(CharSequence token) {
-    return vectors.get(token.toString());
+  public WordVector get(String token) {
+    return vectors.get(token);
   }
 
   @Override
diff --git 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/WordVectorTable.java
 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/WordVectorTable.java
index 17c347a3b..5d5510791 100644
--- 
a/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/WordVectorTable.java
+++ 
b/opennlp-tools/src/main/java/opennlp/tools/util/wordvector/WordVectorTable.java
@@ -28,7 +28,7 @@
 @Experimental
 public interface WordVectorTable {
 
-  WordVector get(CharSequence token);
+  WordVector get(String token);
 
   int size();
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> WordVectorTable should reference WVs by String
> ----------------------------------------------
>
>                 Key: OPENNLP-1169
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1169
>             Project: OpenNLP
>          Issue Type: Bug
>          Components: word vectors
>            Reporter: Tommaso Teofili
>            Assignee: Tommaso Teofili
>             Fix For: 1.8.4
>
>
> {{WordVectorsTable}} API retrieves {{WordVector}} via {{CharSequence}} , this 
> is suboptimal as implementors could store such WVs via an hash table (e.g. 
> {{MapWordVectorsTable}}) and the value of {{CharSequence#toString}} is not 
> guaranteed to be the stable.
> Additionally it's more common to have words as Strings rather than 
> CharSequences, being that more consistent with other OpenNLP APIs (e.g. 
> {{Tokenizer}} ).
> So {{WordVectorsTable}} should instead retrieve {{WordVector}}s using String.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to