[GitHub] [opennlp-sandbox] kinow commented on a diff in pull request #71: Update sandbox component 'opennlp-similarity' to be compatible with latest opennlp-tools release

["kinow (via GitHub)" <gi...@apache.org>]

kinow commented on code in PR #71:
URL: https://github.com/apache/opennlp-sandbox/pull/71#discussion_r1083301371


##########
opennlp-similarity/src/main/java/opennlp/tools/apps/review_builder/WebPageReviewExtractor.java:
##########
@@ -85,7 +85,7 @@ public ReviewObj extractSentencesWithPotentialReviewPhrases(String url)
 					continue;
 				item = item.replace("<span>","").replace("</span>","").replace("<b>","").replace("</b>","");
 				if (item.length()>80 && MinedSentenceProcessor.acceptableMinedSentence(item)==null){
-					System.out.println("Rejected sentence by GeneratedSentenceProcessor.acceptableMinedSentence = "+item);
+					// System.out.println("Rejected sentence by GeneratedSentenceProcessor.acceptableMinedSentence = "+item);

Review Comment:
   Do you think this will be needed at all later? Like if we add logging via slf4j, etc? If so maybe make this and the other commended code TODOs?



##########
opennlp-similarity/src/test/java/opennlp/tools/parse_thicket/apps/MultiSentenceSearchResultsProcessorTest.java:
##########
@@ -20,11 +20,18 @@
 
 import opennlp.tools.similarity.apps.HitBase;
 
-import junit.framework.TestCase;
+import org.junit.Ignore;
+import org.junit.Test;
 
-public class MultiSentenceSearchResultsProcessorTest extends TestCase {
-	MultiSentenceSearchResultsProcessor proc = new MultiSentenceSearchResultsProcessor();
+import static org.junit.Assert.assertTrue;
 
+public class MultiSentenceSearchResultsProcessorTest {
+
+	private final MultiSentenceSearchResultsProcessor proc = new MultiSentenceSearchResultsProcessor();
+
+	@Test
+	@Ignore
+	// This test case fails with: "UnknownHostException: api.datamarket.azure.com: nodename nor servname provided, or not known"

Review Comment:
   :+1: add a TODO here or an issue for later? So we don't forget about it.



##########
opennlp-similarity/src/test/java/opennlp/tools/similarity/apps/taxo_builder/TaxonomyBuildMatchTest.java:
##########
@@ -16,36 +16,38 @@
  */
 package opennlp.tools.similarity.apps.taxo_builder;
 
-import java.util.List;
+import org.junit.Test;
 
-import junit.framework.TestCase;
+import static org.junit.Assert.assertTrue;
 
-public class TaxonomyBuildMatchTest extends TestCase {
+public class TaxonomyBuildMatchTest {
 
+  @Test
   public void testTaxonomySeedImport() {
     AriAdapter ad = new AriAdapter();
     ad.getChainsFromARIfile("src/test/resources/taxonomies/irs_dom.ari");
-    System.out.println(ad.lemma_AssocWords);
     assertTrue(ad.lemma_AssocWords.size() > 0);
   }
-/*
-  public void testTaxonomyBuild() {
-    TaxonomyExtenderViaMebMining self = new TaxonomyExtenderViaMebMining();
-    self.extendTaxonomy("src/test/resources/taxonomies/irs_dom.ari", "tax",
-        "en");
-    self.close();
-    assertTrue(self.getAssocWords_ExtendedAssocWords().size() > 0);
-  }
-*/
+
+  /*
+    public void testTaxonomyBuild() {
+      TaxonomyExtenderViaMebMining self = new TaxonomyExtenderViaMebMining();
+      self.extendTaxonomy("src/test/resources/taxonomies/irs_dom.ari", "tax",
+          "en");
+      self.close();
+      assertTrue(self.getAssocWords_ExtendedAssocWords().size() > 0);
+    }
+  */

Review Comment:
   Should we keep it with an `@Ignore` and todo instead of deleting it? Or was there a reason for doing it this way?



##########
opennlp-similarity/src/main/java/opennlp/tools/similarity/apps/solr/NLProgram2CodeRequestHandler.java:
##########
@@ -93,33 +53,28 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp){
 					text[count] = val;
 					count++;
 				}
+			}
 
+		StringBuilder buf = new StringBuilder();
+		for(String sent:text){
+			ObjectPhraseListForSentence opls=null;
+			try {
+				opls = compiler.convertSentenceToControlObjectPhrase(sent);
+			} catch (Exception e) {
+				e.printStackTrace();
 			}
-		
+			System.out.println(sent+"\n"+opls+"\n");

Review Comment:
   Strange that this class declared a JUL logger, but in end still used sysouts over the code :shrug: 



-- 
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