You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by "rzo1 (via GitHub)" <gi...@apache.org> on 2026/10/02 08:37:12 UTC
[PR] Minor: Share one requireNonNullArg helper across modules (opennlp)
rzo1 opened a new pull request, #1353:
URL: https://github.com/apache/opennlp/pull/1353
Adds an @Internal ArgumentChecks.requireNonNullArg in opennlp-api and replaces the private copies and inline null checks across api, runtime, dl and spellcheck, so all of them report "<name> must not be null". AbstractDL.requireNonNullArg shipped in M5, so it stays as a deprecated delegate.
--
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] Minor: Share one requireNonNullArg helper across modules (opennlp)
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #1353:
URL: https://github.com/apache/opennlp/pull/1353#issuecomment-5954436795
Updated the description. Thx for review.
--
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] Minor: Share one requireNonNullArg helper across modules (opennlp)
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne merged PR #1353:
URL: https://github.com/apache/opennlp/pull/1353
--
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] Minor: Share one requireNonNullArg helper across modules (opennlp)
Posted by "krickert (via GitHub)" <gi...@apache.org>.
krickert commented on PR #1353:
URL: https://github.com/apache/opennlp/pull/1353#issuecomment-5952311589
Overall it's fine - sending as approved assuming you agree w/suggestions.
I tested all claims and edited below.
I pushed one commit for the one leftover in a file already in the diff, the vocab check in `AbstractDL.createWordpieceEncoder`.
Everything checks out:
> Checked all 28 files: each replaced check still throws `IllegalArgumentException`, the return value is used where it feeds super(...), and the tests that pinned the old "The text must not be null." wording are updated. api, runtime, dl, spellcheck and tools pass locally.
Description change is needed:
> The description says that the PR is replacing the inline null checks across api, runtime, dl and spellcheck, but the diff covers the `CharSequenceNormalizer` implementations and the private helper copies. The same packages still have manual checks: `TermAnalyzer`, `Term`, `NormalizationProfiles`, `Confusables`, `EmojiAnnotator` and `EmojiFlags` in normalizer; `StopwordFilteringTokenizer`, `StopwordLists` and `DictionaryStopwordFilter` in stopword; `SentenceVectorsDL`, `OnnxTextEmbedderProvider` and `DocumentCategorizerConfig` in dl. The spellcheck loaders (`FrequencyDictionaryLoader`, `SymSpellModel`, `SymSpellModels`, `SuggestItem`) use `Objects.requireNonNull`, so they throw NPE. Fine to leave those for a follow-up, but then the description should match.
AI suggestion (I agree with it and think it looks good):
> "Adds an `@Internal ArgumentChecks.requireNonNullArg` in `opennlp-api` and replaces the private copies of it in `StringUtil`, `CharClass`, `WordTokenizer`, `AbstractDL`, `StopwordFilterStream` and the spellcheck streams, plus the inline null checks of the `CharSequenceNormalizer` implementations, so all of them report "<name> must not be null". Other manual checks in the same packages are left for a follow-up. `AbstractDL.requireNonNullArg` is part of M5, so it remains as a deprecated delegate."
--
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