You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by "krickert (via GitHub)" <gi...@apache.org> on 2026/09/19 19:42:25 UTC
[PR] OPENNLP-1989: Regex removal (5b/10): Preserve feature identities in event parsing (opennlp)
krickert opened a new pull request, #1321:
URL: https://github.com/apache/opennlp/pull/1321
Replaces regex-based event parsing with shared parsing that follows the text format's delimiters and preserves Unicode feature names. `RealValueFileEventStream` and `RealBasicEventStream` use the same parser, with space, tab, carriage return, line feed, and form feed separating fields, matching `FileEventStream` independently of tokenizer settings. Other characters, including no-break and ideographic spaces, remain in feature names. Leading and repeated separators produce no empty features, outcome-only records remain valid events without ending the stream, and blank records report a format error. `SimpleEventStreamBuilder` uses the same context delimiters, preserves slashes after the outcome separator, and validates valued contexts. Tests and the manual cover these contracts and Unicode edge cases.
JIRA: [OPENNLP-1989](https://issues.apache.org/jira/browse/OPENNLP-1989). Part 5b/10 of the regex-removal work; targets `main` independently.
Validation on Linux with JDK 25:
- Full ML commons and maxent verification passes, with 143 and 51 tests respectively, plus reactor dependencies.
- A separate targeted run passes 127 tests with `opennlp.whitespace.mode=LEGACY`; the default mode passes in the full verification.
- Javadoc and the HTML manual build pass. Local Maven runs used `-Dopennlp.forkCount=1 -Drat.skip=true`; existing unrelated skips and Javadoc warnings remain. Hosted CI has not run for this split.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1989: Regex removal (5b/10): Preserve feature identities in event parsing (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 merged PR #1321:
URL: https://github.com/apache/opennlp/pull/1321
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1989: Regex removal (5b/10): Preserve feature identities in event parsing (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #1321:
URL: https://github.com/apache/opennlp/pull/1321#discussion_r4136825586
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/main/java/opennlp/tools/ml/model/RealValueFileEventStream.java:
##########
@@ -125,21 +131,41 @@ public static float[] parseContexts(String[] contexts) {
return values;
}
+ /**
+ * Parses one event line. Fields are separated by runs of space, tab, carriage return,
+ * line feed and form feed, as in {@link FileEventStream}; the first field is the outcome,
+ * the rest are contexts parsed by {@link #parseContexts(String[])}.
+ *
+ * @param line The event line. Must not be {@code null}.
+ * @return The event; a line with only an outcome gives an event without contexts.
+ * @throws IllegalArgumentException Thrown if {@code line} is {@code null} or a value is negative,
+ * NaN or infinite.
+ * @throws InvalidFormatException Thrown if {@code line} is blank.
+ */
+ public static Event parseEvent(String line) throws InvalidFormatException {
Review Comment:
The vertical tab was a separator here and in `RealBasicEventStream` (`split("\\s+")`) and isn't one anymore. That's only mentioned in a test comment. Please add it to the description next to the outcome-only change.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/test/java/opennlp/tools/ml/AbstractEventStreamTest.java:
##########
@@ -60,6 +60,14 @@ public abstract class AbstractEventStreamTest {
protected static final String EVENTS_INVALID_NEGATIVE =
"other wc=ic=-1.0 w&c=he,ic=-2.0 n1wc=lc=-3.0 n1w&c=belongs,lc=-4.0 n2wc=lc=-5.0\n";
+ /**
+ * Characters that stay inside an event field: Unicode whitespace outside the delimiter set,
+ * the vertical tab, which {@code split("\\s+")} used to treat as a separator, a file separator,
Review Comment:
"which `split("\\s+")` used to treat as a separator" is history. Say what the test checks.
##########
opennlp-docs/src/docbkx/machine-learning.xml:
##########
@@ -86,6 +86,22 @@ under the License.
component which uses this functionality.
</para>
<para>
+ <code>FileEventStream</code> reads events from a text file, one event per line, and
+ <code>RealValueFileEventStream</code> and <code>RealBasicEventStream</code> read events
+ with real-valued features. The first field of a line is the outcome and each further field
+ is a context, optionally followed by <code>=</code> and its value after the last
+ <code>=</code>. Space, tab, carriage return, line feed and form feed separate fields; other
+ characters, including no-break spaces, stay inside feature names. A line with only an
+ outcome is an event without contexts, and a blank line is an error. Since 3.0, an outcome-only line
Review Comment:
Much longer than its neighbours, please rewrap.
##########
opennlp-core/opennlp-ml/opennlp-ml-commons/src/test/java/opennlp/tools/ml/model/RealValueFileEventStreamTest.java:
##########
@@ -92,4 +81,193 @@ void testReset() throws IOException {
} catch (UnsupportedOperationException expected) {
}
}
+
+ @Test
+ void testReadSplitsContextsOnWhitespaceRuns() throws IOException {
+ String input = "other wc=ic=1.0\t\tw&c=he,ic=2.0 n1wc=lc=3.0 \t \n"
+ + "other wc=lc=1.0 w&c=belongs,lc=2.0\u00A0p1wc=ic=3.0\n"
+ + "other wc=lc=1.0 w&c=to,lc=2.0 \n";
+ try (ObjectStream<Event> eventStream = createEventStream(input)) {
+ Event e = eventStream.read();
+ Assertions.assertArrayEquals(
+ new String[] {"wc=ic", "w&c=he,ic", "n1wc=lc"}, e.getContext());
+ Assertions.assertArrayEquals(new float[] {1.0f, 2.0f, 3.0f}, e.getValues());
+ e = eventStream.read();
+ Assertions.assertArrayEquals(
+ new String[] {"wc=lc", "w&c=belongs,lc=2.0\u00A0p1wc=ic"}, e.getContext());
+ Assertions.assertArrayEquals(new float[] {1.0f, 3.0f}, e.getValues());
+ // repeated runs produce no empty predicate
+ e = eventStream.read();
+ Assertions.assertArrayEquals(
+ new String[] {"wc=lc", "w&c=to,lc"}, e.getContext());
+ Assertions.assertArrayEquals(new float[] {1.0f, 2.0f}, e.getValues());
+ Assertions.assertNull(eventStream.read());
+ }
+ }
+
+ private static Stream<Arguments> fieldSeparators() {
+ return Stream.of(
+ // the fixed event separators, including runs and leading separators
+ Arguments.of("other\twc=ic=1.0", "other", new String[] {"wc=ic"}),
+ Arguments.of("other \t wc=ic=1.0", "other", new String[] {"wc=ic"}),
+ Arguments.of("other\fwc=ic=1.0", "other", new String[] {"wc=ic"}),
+ Arguments.of(" other wc=ic=1.0", "other", new String[] {"wc=ic"}),
+ Arguments.of("\tother wc=ic=1.0", "other", new String[] {"wc=ic"}),
+ Arguments.of("other wc=ic=1.0\r", "other", new String[] {"wc=ic"}),
+ // non-delimiter characters remain part of the outcome
+ Arguments.of("other\u00A0wc=ic=1.0", "other\u00A0wc=ic=1.0", new String[0]),
+ Arguments.of("other\u0085wc=ic=1.0", "other\u0085wc=ic=1.0", new String[0]),
+ Arguments.of("other\u2028wc=ic=1.0", "other\u2028wc=ic=1.0", new String[0]),
+ Arguments.of("other\u3000wc=ic=1.0", "other\u3000wc=ic=1.0", new String[0]),
+ // file separator and zero width space are not delimiters either
+ Arguments.of("other\u001Cwc=ic=1.0", "other\u001Cwc=ic=1.0", new String[0]),
+ Arguments.of("other\u200Bwc=ic=1.0", "other\u200Bwc=ic=1.0", new String[0]),
+ Arguments.of("other wc\u001C=ic=1.0", "other", new String[] {"wc\u001C=ic"}),
+ // a supplementary character is one character of a field
+ Arguments.of("\uD83D\uDE00 w=\uD83D\uDE00=1.0", "\uD83D\uDE00", new String[] {"w=\uD83D\uDE00"}));
+ }
+
+ @ParameterizedTest
+ @MethodSource("fieldSeparators")
+ void testFieldsUseEventDelimiters(String line, String outcome, String[] contexts)
+ throws IOException {
+ Event e = RealValueFileEventStream.parseEvent(line);
+ Assertions.assertEquals(outcome, e.getOutcome());
+ Assertions.assertArrayEquals(contexts, e.getContext());
+ }
+
+ @Test
+ void testParseEventRejectsNull() {
+ IllegalArgumentException e = Assertions.assertThrows(IllegalArgumentException.class,
+ () -> RealValueFileEventStream.parseEvent(null));
+ Assertions.assertEquals("line must not be null", e.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"other", "other ", " other\t"})
+ void testOutcomeOnlyLineHasNoContexts(String line) throws IOException {
+ Event e = RealValueFileEventStream.parseEvent(line);
+ Assertions.assertEquals("other", e.getOutcome());
+ Assertions.assertEquals(0, e.getContext().length);
+ Assertions.assertNull(e.getValues());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"", " ", "\t", "\f", " \t\r\n\f", "\r"})
+ void testLineWithoutOutcomeIsRejected(String line) {
+ InvalidFormatException e = Assertions.assertThrows(InvalidFormatException.class,
+ () -> RealValueFileEventStream.parseEvent(line));
+ Assertions.assertEquals("An event line must start with an outcome: \"" + line + "\"", e.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"org/start", "a=b", "x;y", "B-ORG", "\u00E9tat", "\uD83D\uDE00"})
+ void testOutcomeMayContainAnyNonWhitespace(String outcome) throws IOException {
+ Event e = RealValueFileEventStream.parseEvent(outcome + " wc=ic=1.0");
+ Assertions.assertEquals(outcome, e.getOutcome());
+ Assertions.assertArrayEquals(new String[] {"wc=ic"}, e.getContext());
+ }
+
+ private static Stream<Arguments> contextValues() {
+ return Stream.of(
+ Arguments.of("w&c=he,ic=2.0", "w&c=he,ic", 2.0f),
+ Arguments.of("a=b=c=3.5", "a=b=c", 3.5f),
+ Arguments.of("wc=ic=1e2", "wc=ic", 100f),
+ Arguments.of("wc=ic=1E-2", "wc=ic", 0.01f),
+ Arguments.of("wc=ic=.5", "wc=ic", 0.5f),
+ Arguments.of("wc=ic=+2", "wc=ic", 2f),
+ Arguments.of("wc=ic=0", "wc=ic", 0f));
+ }
+
+ /** The value follows the last {@code =}, so a context name may contain the character. */
+ @ParameterizedTest
+ @MethodSource("contextValues")
+ void testContextValueFollowsTheLastEqualsSign(String context, String name, float value)
+ throws IOException {
+ Event e = RealValueFileEventStream.parseEvent("other " + context);
+ Assertions.assertArrayEquals(new String[] {name}, e.getContext());
+ Assertions.assertArrayEquals(new float[] {value}, e.getValues());
+ }
+
+ @ParameterizedTest
+ // no equals sign, an empty value after it, an equals sign in first position, text or a comma after it
+ @ValueSource(strings = {"wc", "wc=", "=5", "wc=abc", "wc=1,5", "wc=ic=1.0.0"})
+ void testContextWithoutANumberIsKeptWholeAndCountsOnce(String context) throws IOException {
+ Event e = RealValueFileEventStream.parseEvent("other " + context);
+ Assertions.assertArrayEquals(new String[] {context}, e.getContext());
+ Assertions.assertNull(e.getValues());
+ }
+
+ @Test
+ void testContextsWithAndWithoutValuesMix() throws IOException {
+ Event e = RealValueFileEventStream.parseEvent("other wc=ic=2.0 wc n1wc=lc=0.5");
+ Assertions.assertArrayEquals(new String[] {"wc=ic", "wc", "n1wc=lc"}, e.getContext());
+ Assertions.assertArrayEquals(new float[] {2.0f, 1.0f, 0.5f}, e.getValues());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"wc=ic=-1", "wc=ic=-0.5", "wc=ic=-1e2", "wc=ic=-1E-2"})
+ void testNegativeValueIsRejectedWithTheContextNamed(String context) {
+ IllegalArgumentException e = Assertions.assertThrows(IllegalArgumentException.class,
+ () -> RealValueFileEventStream.parseEvent("other " + context));
+ Assertions.assertEquals("Negative values are not allowed: " + context, e.getMessage());
+ }
+
+ /** {@code Float.parseFloat} accepts these, but they are no usable feature values. */
+ @ParameterizedTest
+ @ValueSource(strings = {"wc=ic=NaN", "wc=ic=Infinity", "wc=ic=-Infinity"})
+ void testNonFiniteValueIsRejectedWithTheContextNamed(String context) {
+ IllegalArgumentException e = Assertions.assertThrows(IllegalArgumentException.class,
+ () -> RealValueFileEventStream.parseEvent("other " + context));
+ Assertions.assertEquals("Values must be finite: " + context, e.getMessage());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"\n", "\r\n", "\r"})
+ void testReadAcceptsEveryLineTerminator(String terminator) throws IOException {
+ String input = "other wc=ic=1.0" + terminator + "other wc=lc=2.0" + terminator;
+ try (ObjectStream<Event> eventStream = createEventStream(input)) {
+ Assertions.assertArrayEquals(new String[] {"wc=ic"}, eventStream.read().getContext());
+ Assertions.assertArrayEquals(new String[] {"wc=lc"}, eventStream.read().getContext());
+ Assertions.assertNull(eventStream.read());
+ }
+ }
+
+ @Test
+ void testReadRejectsBlankLine() throws IOException {
+ try (ObjectStream<Event> eventStream = createEventStream("other wc=ic=1.0\n\nother wc=lc=1.0\n")) {
+ Assertions.assertEquals("other", eventStream.read().getOutcome());
+ Assertions.assertThrows(InvalidFormatException.class, eventStream::read);
+ }
+ }
+
+ /** See {@link #NON_DELIMITER_CHARS}: only the vertical tab row pins a change from 2.x. */
Review Comment:
"pins a change from 2.x" is history. Say what the test checks.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1989: Regex removal (5b/10): Preserve feature identities in event parsing (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on PR #1321:
URL: https://github.com/apache/opennlp/pull/1321#issuecomment-5881987260
Changes pushed:
- `FileEventStream` uses the shared split to patch javadoc.
- `NumberFormatException` is wrapped in `IllegalArgumentException`, and a negative value throws `IllegalArgumentException` in `RealValueFileEventStream` too, with `@throws` updated. (matches our IAE rule on nulls)
- `parseEvent` Javadoc now describes the method’s contract. Removed the whitespace mode discussion from both the Javadoc and the manual.
- Shortened the manual to explain the format with one example and note the outcome-only behavior change since 3.0.
- Moved the separator characters and format message into constants.
- Rewrote the PR description and removed the CI note and AI text.
Once #1278 is merged, I’ll update this PR and #1320 to use `StringUtil.splitNonEmpty`.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OPENNLP-1989: Regex removal (5b/10): Preserve feature identities in event parsing (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on PR #1321:
URL: https://github.com/apache/opennlp/pull/1321#issuecomment-5902773641
Fixed the two test comments and rewrapped the manual paragraph. The description now names the vertical tab change next to the outcome-only one.
--
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: commits-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org