[jira] [Commented] (OPENNLP-1214) use hash to avoid linear search in DefaultEndOfSentenceScanner
[ https://issues.apache.org/jira/browse/OPENNLP-1214?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16579238#comment-16579238 ] ASF GitHub Bot commented on OPENNLP-1214: - kojisekig opened a new pull request #329: OPENNLP-1214: use hash to avoid linear search in DefaultEndOfSentence… URL: https://github.com/apache/opennlp/pull/329 …Scanner 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: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with OPENNLP- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp 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](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp 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 travis-ci 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > use hash to avoid linear search in DefaultEndOfSentenceScanner > -- > > Key: OPENNLP-1214 > URL: https://issues.apache.org/jira/browse/OPENNLP-1214 > Project: OpenNLP > Issue Type: Improvement >Affects Versions: 1.9.0 >Reporter: Koji Sekiguchi >Priority: Minor > Fix For: 1.9.1 > > > When DefaultEndOfSentenceScanner scans a sentence, it uses linear search to > check if each characters in the sentence is one of eos characters. I think > we'd better use HashSet to keep eosCharacters instead of char[]. > In accordance with this replacement, I'd like to make > getEndOfSentenceCharacters() deprecated because it returns char[] and nobody > in OpenNLP calls it at present, and I'd like to add the equivalent method > which returns Set of eos chars. Though it cannot keep the order of > eos chars but I don't think it can be a problem anyway. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (OPENNLP-1212) TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag
[ https://issues.apache.org/jira/browse/OPENNLP-1212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Koji Sekiguchi resolved OPENNLP-1212. - Resolution: Fixed > TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag > --- > > Key: OPENNLP-1212 > URL: https://issues.apache.org/jira/browse/OPENNLP-1212 > Project: OpenNLP > Issue Type: Bug >Affects Versions: 1.9.0 >Reporter: Koji Sekiguchi >Assignee: Koji Sekiguchi >Priority: Minor > Fix For: 1.9.1 > > > As TokenFeatureGenerator can accept lowercase flag but > TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag, > TokenFeatureGenerator always return lowercase tokens. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Resolved] (OPENNLP-1211) Improve WindowFeatureGeneratorTest
[
https://issues.apache.org/jira/browse/OPENNLP-1211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Koji Sekiguchi resolved OPENNLP-1211.
-
Resolution: Fixed
> Improve WindowFeatureGeneratorTest
> --
>
> Key: OPENNLP-1211
> URL: https://issues.apache.org/jira/browse/OPENNLP-1211
> Project: OpenNLP
> Issue Type: Test
> Components: Build, Packaging and Test
>Affects Versions: 1.9.0
>Reporter: Koji Sekiguchi
>Assignee: Koji Sekiguchi
>Priority: Trivial
> Fix For: 1.9.1
>
>
> I'd like to improve WindowFeatureGeneratorTest from the following perspective:
> * testWindowSizeOne should check the contents of the returned features. It
> checks the length of the features only now
> * most of test methods uses Assert.assertEquals(expected, actual) in opposite
> way for its arguments when checking the contents of the returned features
> {code}
> Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
> {code}
> should be
> {code}
> Assert.assertEquals(testSentence[testTokenIndex], features.get(0));
> {code}
> * Though I pointed out the arguments in assertEquals() above, I think we'd
> better use exact concrete string rather than expression such like
> testSentence[testTokenIndex] for the expected. And also,
> testForCorrectFeatures uses contains method when checking the contents of the
> returned features but I think we should avoid using contains when checking
> the items in a List, rather than writing like this:
> {code}
> Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
> "2" +
> testSentence[testTokenIndex - 2]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
> "1" +
> testSentence[testTokenIndex - 1]));
> Assert.assertTrue(features.contains(testSentence[testTokenIndex]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
> "1" +
> testSentence[testTokenIndex + 1]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
> "2" +
> testSentence[testTokenIndex + 2]));
> {code}
> but I'd like to rewrite them like this:
> {code}
> Assert.assertEquals("d",features.get(0));
> Assert.assertEquals("p1c",features.get(1));
> Assert.assertEquals("p2b",features.get(2));
> Assert.assertEquals("n1e",features.get(3));
> Assert.assertEquals("n2f",features.get(4));
> {code}
> The second form helps us to understand how WindowFeatureGenerator works and
> it's easier to read.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Assigned] (OPENNLP-1211) Improve WindowFeatureGeneratorTest
[
https://issues.apache.org/jira/browse/OPENNLP-1211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Koji Sekiguchi reassigned OPENNLP-1211:
---
Assignee: Koji Sekiguchi
> Improve WindowFeatureGeneratorTest
> --
>
> Key: OPENNLP-1211
> URL: https://issues.apache.org/jira/browse/OPENNLP-1211
> Project: OpenNLP
> Issue Type: Test
> Components: Build, Packaging and Test
>Affects Versions: 1.9.0
>Reporter: Koji Sekiguchi
>Assignee: Koji Sekiguchi
>Priority: Trivial
> Fix For: 1.9.1
>
>
> I'd like to improve WindowFeatureGeneratorTest from the following perspective:
> * testWindowSizeOne should check the contents of the returned features. It
> checks the length of the features only now
> * most of test methods uses Assert.assertEquals(expected, actual) in opposite
> way for its arguments when checking the contents of the returned features
> {code}
> Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
> {code}
> should be
> {code}
> Assert.assertEquals(testSentence[testTokenIndex], features.get(0));
> {code}
> * Though I pointed out the arguments in assertEquals() above, I think we'd
> better use exact concrete string rather than expression such like
> testSentence[testTokenIndex] for the expected. And also,
> testForCorrectFeatures uses contains method when checking the contents of the
> returned features but I think we should avoid using contains when checking
> the items in a List, rather than writing like this:
> {code}
> Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
> "2" +
> testSentence[testTokenIndex - 2]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
> "1" +
> testSentence[testTokenIndex - 1]));
> Assert.assertTrue(features.contains(testSentence[testTokenIndex]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
> "1" +
> testSentence[testTokenIndex + 1]));
> Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
> "2" +
> testSentence[testTokenIndex + 2]));
> {code}
> but I'd like to rewrite them like this:
> {code}
> Assert.assertEquals("d",features.get(0));
> Assert.assertEquals("p1c",features.get(1));
> Assert.assertEquals("p2b",features.get(2));
> Assert.assertEquals("n1e",features.get(3));
> Assert.assertEquals("n2f",features.get(4));
> {code}
> The second form helps us to understand how WindowFeatureGenerator works and
> it's easier to read.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Assigned] (OPENNLP-1212) TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag
[ https://issues.apache.org/jira/browse/OPENNLP-1212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Koji Sekiguchi reassigned OPENNLP-1212: --- Assignee: Koji Sekiguchi > TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag > --- > > Key: OPENNLP-1212 > URL: https://issues.apache.org/jira/browse/OPENNLP-1212 > Project: OpenNLP > Issue Type: Bug >Affects Versions: 1.9.0 >Reporter: Koji Sekiguchi >Assignee: Koji Sekiguchi >Priority: Minor > Fix For: 1.9.1 > > > As TokenFeatureGenerator can accept lowercase flag but > TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag, > TokenFeatureGenerator always return lowercase tokens. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (OPENNLP-1214) use hash to avoid linear search in DefaultEndOfSentenceScanner
Koji Sekiguchi created OPENNLP-1214: --- Summary: use hash to avoid linear search in DefaultEndOfSentenceScanner Key: OPENNLP-1214 URL: https://issues.apache.org/jira/browse/OPENNLP-1214 Project: OpenNLP Issue Type: Improvement Affects Versions: 1.9.0 Reporter: Koji Sekiguchi Fix For: 1.9.1 When DefaultEndOfSentenceScanner scans a sentence, it uses linear search to check if each characters in the sentence is one of eos characters. I think we'd better use HashSet to keep eosCharacters instead of char[]. In accordance with this replacement, I'd like to make getEndOfSentenceCharacters() deprecated because it returns char[] and nobody in OpenNLP calls it at present, and I'd like to add the equivalent method which returns Set of eos chars. Though it cannot keep the order of eos chars but I don't think it can be a problem anyway. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OPENNLP-1213) Use ja for Japanese language code rather than jp
[ https://issues.apache.org/jira/browse/OPENNLP-1213?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16579183#comment-16579183 ] ASF GitHub Bot commented on OPENNLP-1213: - kojisekig opened a new pull request #328: OPENNLP-1213: Use ja for Japanese language code rather than jp URL: https://github.com/apache/opennlp/pull/328 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: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Does your PR title start with OPENNLP- where is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [ ] Has your PR been rebased against the latest commit within the target branch (typically master)? - [ ] Is your initial contribution a single, squashed commit? ### For code changes: - [ ] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp 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](http://www.apache.org/legal/resolved.html#category-a)? - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder? - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp 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 travis-ci 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 GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Use ja for Japanese language code rather than jp > > > Key: OPENNLP-1213 > URL: https://issues.apache.org/jira/browse/OPENNLP-1213 > Project: OpenNLP > Issue Type: Bug >Affects Versions: 1.9.0 >Reporter: Koji Sekiguchi >Priority: Minor > Fix For: 1.9.1 > > > It seems that Factory of sentdetect uses "jp" for Japanese language code but > I think it is country code. Let's use "ja" instead. > We could leave "jp" for back-compat, but I don't think we need to do it. So > I'll just replace "jp" with "ja" in the patch. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (OPENNLP-1213) Use ja for Japanese language code rather than jp
Koji Sekiguchi created OPENNLP-1213: --- Summary: Use ja for Japanese language code rather than jp Key: OPENNLP-1213 URL: https://issues.apache.org/jira/browse/OPENNLP-1213 Project: OpenNLP Issue Type: Bug Affects Versions: 1.9.0 Reporter: Koji Sekiguchi Fix For: 1.9.1 It seems that Factory of sentdetect uses "jp" for Japanese language code but I think it is country code. Let's use "ja" instead. We could leave "jp" for back-compat, but I don't think we need to do it. So I'll just replace "jp" with "ja" in the patch. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (OPENNLP-1212) TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag
[
https://issues.apache.org/jira/browse/OPENNLP-1212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16578978#comment-16578978
]
ASF GitHub Bot commented on OPENNLP-1212:
-
kojisekig closed pull request #327: OPENNLP-1212: TokenFeatureGeneratorFactory
doesn't allow us to set lo…
URL: https://github.com/apache/opennlp/pull/327
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/featuregen/TokenFeatureGeneratorFactory.java
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/TokenFeatureGeneratorFactory.java
index 20612f56d..666030516 100644
---
a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/TokenFeatureGeneratorFactory.java
+++
b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/TokenFeatureGeneratorFactory.java
@@ -44,6 +44,6 @@ static void register(Map fa
@Override
public AdaptiveFeatureGenerator create() throws InvalidFormatException {
-return new TokenFeatureGenerator();
+return new TokenFeatureGenerator(getBool("lowercase", true));
}
}
diff --git
a/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
b/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
index 1f60ad18b..d95549f4a 100644
---
a/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
+++
b/opennlp-tools/src/main/resources/opennlp/tools/namefind/ner-default-features.xml
@@ -27,7 +27,9 @@
2
2
-
+
+ true
+
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:
us...@infra.apache.org
> TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag
> ---
>
> Key: OPENNLP-1212
> URL: https://issues.apache.org/jira/browse/OPENNLP-1212
> Project: OpenNLP
> Issue Type: Bug
>Affects Versions: 1.9.0
>Reporter: Koji Sekiguchi
>Priority: Minor
> Fix For: 1.9.1
>
>
> As TokenFeatureGenerator can accept lowercase flag but
> TokenFeatureGeneratorFactory doesn't allow us to set lowercase flag,
> TokenFeatureGenerator always return lowercase tokens.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (OPENNLP-1211) Improve WindowFeatureGeneratorTest
[
https://issues.apache.org/jira/browse/OPENNLP-1211?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16578976#comment-16578976
]
ASF GitHub Bot commented on OPENNLP-1211:
-
kojisekig closed pull request #326: OPENNLP-1211: Improve
WindowFeatureGeneratorTest
URL: https://github.com/apache/opennlp/pull/326
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/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
index aff43c06b..12af2c8ef 100644
---
a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
+++
b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/WindowFeatureGeneratorTest.java
@@ -55,7 +55,7 @@ public void testWithoutWindow() {
Assert.assertEquals(1, features.size());
-Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+Assert.assertEquals("c", features.get(0));
}
@Test
@@ -68,6 +68,10 @@ public void testWindowSizeOne() {
windowFeatureGenerator.createFeatures(features, testSentence,
testTokenIndex, null);
Assert.assertEquals(3, features.size());
+
+Assert.assertEquals("c", features.get(0));
+Assert.assertEquals("p1b", features.get(1));
+Assert.assertEquals("n1d", features.get(2));
}
@Test
@@ -78,7 +82,7 @@ public void testWindowAtBeginOfSentence() {
int testTokenIndex = 0;
windowFeatureGenerator.createFeatures(features, testSentence,
testTokenIndex, null);
Assert.assertEquals(1, features.size());
-Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+Assert.assertEquals("a", features.get(0));
}
@Test
@@ -89,7 +93,7 @@ public void testWindowAtEndOfSentence() {
int testTokenIndex = testSentence.length - 1;
windowFeatureGenerator.createFeatures(features, testSentence,
testTokenIndex, null);
Assert.assertEquals(1, features.size());
-Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
+Assert.assertEquals("h", features.get(0));
}
/**
@@ -104,16 +108,10 @@ public void testForCorrectFeatures() {
windowFeatureGenerator.createFeatures(features, testSentence,
testTokenIndex, null);
Assert.assertEquals(5, features.size());
-Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
"2" +
-testSentence[testTokenIndex - 2]));
-Assert.assertTrue(features.contains(WindowFeatureGenerator.PREV_PREFIX +
"1" +
-testSentence[testTokenIndex - 1]));
-
-Assert.assertTrue(features.contains(testSentence[testTokenIndex]));
-
-Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
"1" +
-testSentence[testTokenIndex + 1]));
-Assert.assertTrue(features.contains(WindowFeatureGenerator.NEXT_PREFIX +
"2" +
-testSentence[testTokenIndex + 2]));
+Assert.assertEquals("d", features.get(0));
+Assert.assertEquals("p1c", features.get(1));
+Assert.assertEquals("p2b", features.get(2));
+Assert.assertEquals("n1e", features.get(3));
+Assert.assertEquals("n2f", features.get(4));
}
}
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:
us...@infra.apache.org
> Improve WindowFeatureGeneratorTest
> --
>
> Key: OPENNLP-1211
> URL: https://issues.apache.org/jira/browse/OPENNLP-1211
> Project: OpenNLP
> Issue Type: Test
> Components: Build, Packaging and Test
>Affects Versions: 1.9.0
>Reporter: Koji Sekiguchi
>Priority: Trivial
> Fix For: 1.9.1
>
>
> I'd like to improve WindowFeatureGeneratorTest from the following perspective:
> * testWindowSizeOne should check the contents of the returned features. It
> checks the length of the features only now
> * most of test methods uses Assert.assertEquals(expected, actual) in opposite
> way for its arguments when checking the contents of the returned features
> {code}
> Assert.assertEquals(features.get(0), testSentence[testTokenIndex]);
> {code}
> should be
> {code}
> Assert.assertEquals(testSentence[testTokenIndex], features.get(0));
> {code}
> * Though I pointed out the arguments in assertEquals() above, I think we'd
> better use exact concrete string rather than expression such like
> testSentence[testTokenIndex] for the expected. And also,
>
