Skip to content

OPENNLP-1973: Regex removal (prep): Add a CompatibilityMode switch for the earlier output - #1312

Merged
rzo1 merged 2 commits into
mainfrom
OPENNLP-1973-compatibility-mode
Sep 18, 2026
Merged

rzo1 merged 2 commits into
mainfrom
OPENNLP-1973-compatibility-mode

Conversation

@rzo1

@rzo1 rzo1 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Prerequisite for the regex removal (OPENNLP-1926). Split out of #1275.

Adds opennlp.tools.util.CompatibilityMode to opennlp-api with LEGACY and CURRENT, resolved from -Dopennlp.compat.mode at startup and overridable with setActive and reset. CURRENT is the default; LEGACY logs one warning per process. Built like the whitespace mode switch and scheduled for removal in 4.0 with it.

Classes whose output was corrected consult it so that models trained with an earlier release still get the input they were trained on. Users: the EmojiCharSequenceNormalizer fix (#1313, part 8, OPENNLP-1974, stacked on this PR) and the surrogate rule of #1281 (OPENNLP-1934).

Commits by @krickert from #1275, regrouped.

CompatibilityMode holds LEGACY or CURRENT, resolved from the system
property opennlp.compat.mode and overridable with setActive and reset.
Classes whose output was corrected consult it so models trained with an
earlier release keep their input. Built like the whitespace mode switch
and scheduled for removal in 4.0.
@krickert

Copy link
Copy Markdown
Contributor

Thank you for doing this!

@krickert krickert 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.

Looks good as-is.

Notes from AI but it's very nit:

  • Invalid property values fail at class initialization. fromProperty() runs in the static initializer, so -Dopennlp.compat.mode=bogus surfaces as an ExceptionInInitializerError on first use and NoClassDefFoundError after. WhitespaceMode behaves the same, so this is consistent, and the test deliberately initializes with a clean property. A sentence in the Javadoc of MODE_PROPERTY saying the failure happens at startup would make that visible.
  • The two mode enums duplicate fromProperty and warnIfLegacy. A tiny package-private helper would remove it, but that is a preference and touches a class already on main.

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

Ack, +1

@rzo1

rzo1 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! Added the sentence on class initialization to the MODE_PROPERTY Javadoc. Merging fromProperty/warnIfLegacy with WhitespaceMode would touch a class already on main, so I'd leave that for a separate cleanup.

@rzo1
rzo1 merged commit c47eeaf into main Sep 18, 2026
10 checks passed
@rzo1
rzo1 deleted the OPENNLP-1973-compatibility-mode branch October 3, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants