Visitar URL original
GH-1420: Remove duplication and add readResolve in opennlp-runtime by rzo1 · Pull Request #1427 · apache/opennlp · GitHub
Skip to content

GH-1420: Remove duplication and add readResolve in opennlp-runtime - #1427

Merged
mawiesne merged 2 commits into
mainfrom
gh-1420-runtime-cleanup
Oct 9, 2026
Merged

GH-1420: Remove duplication and add readResolve in opennlp-runtime#1427
mawiesne merged 2 commits into
mainfrom
gh-1420-runtime-cleanup

Conversation

@rzo1

@rzo1 rzo1 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Adds readResolve to the Serializable normalizer singletons so they stay singletons after deserialization. Several local copies of helpers in opennlp-runtime main and test code are replaced with calls to the existing shared ones, such as the regional indicator check, the Span offset constructor and the test fixtures. A POS dictionary entry with no tags attribute now fails with InvalidFormatException, and repeated spaces in tags are handled as before.

Fixes #1420.

@rzo1 rzo1 added this to the 3.0.0 milestone Oct 6, 2026
@rzo1 rzo1 added task Maintenance or housekeeping work (Jira: Task) component: normalizer Text normalization and Unicode data component: tokenizer Tokenizers (UAX 29, lattice, WordPiece, ME) labels Oct 6, 2026
@rzo1 rzo1 added the breaking change Changes public API or behavior; needs a release note label Oct 6, 2026
@rzo1
GH-1420: Remove duplication and add readResolve in opennlp-runtime
77fdb19 
Adds readResolve to the normalizer singletons, reuses shared helpers in place of
local copies in main and test code, and rejects POS dictionary entries without tags.
@rzo1
rzo1 force-pushed the gh-1420-runtime-cleanup branch from e189ea2 to 77fdb19 Compare October 7, 2026 19:14
krickert
krickert approved these changes Oct 8, 2026
View reviewed changes
Comment thread opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/postag/POSDictionaryTest.java Outdated
mawiesne
mawiesne requested changes Oct 8, 2026
View reviewed changes

@mawiesne mawiesne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the PR. The idea, changes and simplifications are clear and correct. Got some final polishing comments (three). Should be quickly adapted / clarified.

Comment thread ...me/src/main/java/opennlp/tools/util/normalizer/ConfusableSkeletonCharSequenceNormalizer.java Outdated
Comment thread ...-runtime/src/main/java/opennlp/tools/util/normalizer/GermanUmlautCharSequenceNormalizer.java Outdated
Comment thread opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/document/TestComponents.java Outdated
Comment thread opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/postag/POSDictionaryTest.java Outdated
@rzo1
GH-1420: Address review: keep normalizers non-final, rename DocumentT…
5574959 
…estStubs, drop the empty-tag test

Reverts final on the two normalizers, renames TestComponents to DocumentTestStubs and removes the POSDictionary test that pinned an empty tag for repeated spaces.
@rzo1
rzo1 requested review from krickert and mawiesne October 9, 2026 07:30
@mawiesne
mawiesne merged commit eef3909 into main Oct 9, 2026
10 checks passed
@mawiesne
mawiesne deleted the gh-1420-runtime-cleanup branch October 9, 2026 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mawiesne mawiesne mawiesne approved these changes
@jzonthemtn jzonthemtn Awaiting requested review from jzonthemtn
@krickert krickert Awaiting requested review from krickert

Assignees

@rzo1 rzo1

Labels

breaking change Changes public API or behavior; needs a release note component: normalizer Text normalization and Unicode data component: tokenizer Tokenizers (UAX 29, lattice, WordPiece, ME) task Maintenance or housekeeping work (Jira: Task)

Projects

None yet

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

Remove duplication and fix serialization gaps in opennlp-runtime found in the 3.0.0 code review

3 participants

@rzo1 @krickert @mawiesne