krickert commented on code in PR #1150:
URL: https://github.com/apache/opennlp/pull/1150#discussion_r3569101810
##########
opennlp-api/src/main/java/opennlp/tools/tokenize/WhitespaceTokenizer.java:
##########
@@ -79,8 +83,12 @@ public Span[] tokenizePos(String d) {
return tokens.toArray(new Span[0]);
}
+ /**
+ * Checks whether {@code character} is a line separator: LF, CR, NEL, LS, or
PS.
+ */
private boolean isLineSeparator(char character) {
Review Comment:
Agreed. Hoisted to StringUtil.isLineSeparator(char); both tokenizers
delegate to it now.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/tokenize/SimpleTokenizer.java:
##########
@@ -106,8 +111,12 @@ else if (Character.isDigit(c)) {
return tokens.toArray(new Span[0]);
}
+ /**
+ * Checks whether {@code character} is a line separator: LF, CR, NEL, LS, or
PS.
+ */
private boolean isLineSeparator(char character) {
- return character == Character.LINE_SEPARATOR || character ==
Character.LETTER_NUMBER;
+ return character == '\n' || character == '\r'
Review Comment:
Same, this one delegates to StringUtil.isLineSeparator(char) as well.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/tokenize/DefaultTokenContextGenerator.java:
##########
@@ -114,6 +114,11 @@ protected List<String> createContext(String sentence, int
index) {
* a fixed text sequence depending on {@code c}. The resulting combination
is added
* to the given list {@code preds}.
*
+ * <p>
+ * The {@code _ws} class deliberately stays on the frozen
Review Comment:
Reworded: the javadoc now says the predicate keeps the whitespace definition
of OpenNLP 1.x and 2.x because the feature strings are part of every trained
tokenizer model. The StringUtil.isWhitespace javadoc got the same rewording.
##########
opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/sentdetect/DefaultSDContextGenerator.java:
##########
@@ -27,7 +27,10 @@
/**
* Generate event contexts for maxent decisions for sentence detection.
- *
+ * <p>
+ * The whitespace checks deliberately stay on the frozen {@link
StringUtil#isWhitespace(char)}
Review Comment:
Dropped.
##########
opennlp-api/src/main/java/opennlp/tools/tokenize/BertNormalization.java:
##########
@@ -62,6 +62,9 @@ static boolean isControl(int codePoint) {
/**
* A whitespace character in the BERT sense: space, tab, newline, carriage
* return, or Unicode space separators ({@code Zs}).
+ *
+ * <p>This definition is deliberately not the Unicode {@code White_Space}
set and stays
Review Comment:
Dropped the second sentence; the first one stands on its own.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
