Repository navigation
Conversation
aaf602d to
fe19155
Compare
…and document the limits of the default The default normalizer and the language detector chain are unchanged, and the class Javadoc states the limits found with the WHATWG URL test data. The new class removes http and https URLs with ports, userinfo, percent escapes, IPv6 hosts and non-ASCII paths, bounded by whitespace and delimiters, with a left boundary before the scheme, in linear time. The manual describes both and how to opt in.
fe19155 to
5a5fd37
Compare
rzo1
left a comment
There was a problem hiding this comment.
The default UrlCharSequenceNormalizer output is wrong (ports, userinfo, git+https), so please fix it in place instead of adding BoundedUrlCharSequenceNormalizer as a new public class nobody gets by default, and keep the old output under CompatibilityMode.LEGACY from #1312, the same way #1313 handles the emoji normalizer. That also means rebasing onto #1312, running the langdetect eval build before merge, and retitling to the user-visible change, e.g. "UrlCharSequenceNormalizer removes whole URLs including ports, userinfo and non-ASCII hosts". A trailing ASCII apostrophe is currently swallowed with the URL ('http://example.com/a' gives "' "), so please give it back like the other trailing punctuation and pin it with a test. Please reuse StringUtil.isAsciiLetter from #1311 instead of the private copy, add full Javadoc to isHttpScheme, and move ASCII_ALNUM out of MailAddressScan to a neutral place. Finally, drop the "(OPENNLP-1946) … kept as they are" history from the Javadoc and tests, and rewrite the langdetect.xml row in the style of the emoji row from #1313.
|
Closed in favor of #1315, which applies the review here: |
|
Danke, @rzo1 |
Based on main, independent of the OPENNLP-1928 stack.
UrlCharSequenceNormalizeris part of the default language detector chain, so its output is part of the features existing models were trained on. It stays as it is. Its Javadoc now states the three limits found with the WHATWG URL test data, and a test table records the current output on those inputs:http://f:21/gives" :21/",http://example.com/%20foogives" %20foo", andhttp://[::1]/is untouched.git+https://github.com/foo/bargives"git+ ".telnet://user:pass@foobar.com:23/gives"telnet://user: :23/".The email scan moved to a package-private helper,
MailAddressScan, that both normalizers use; the default's output and itsserialVersionUID(equal to theserialvervalue before and after) are unchanged.The opt-in variant
BoundedUrlCharSequenceNormalizer, a separate final class withgetInstance()andreadResolve, not in the default chain. A separate class rather than a flag on the existing one, so the serialized form and the behavior of the class inside trained models do not change, and opting in is passing the other instance to the context generator. Rules, all cursor scans by code point:httporhttpsin any letter case, followed by://, at the start of the text or after a code point that is neither a letter, a digit, nor one of_ + - . :. Sogit+https://andblob:https://are not URLs of their own, and neither issee:http://.< > ". It includes a port, userinfo, percent escapes, a bracketed IPv6 host, and non-ASCII host names and paths, supplementary characters included.. , ; : ! ?) and a closing bracket without its opening bracket inside the body are given back (the bracket balance is counted once per URL, so a long run of closers costs linear time), so(http://example.com/a).gives"( )."andhttp://example.com/a(b)is removed in full.telnet://,redis://,ftp://, ...) is left as it is, and no email address is matched inside it. Email addresses elsewhere are matched as in the default.Out of scope, as in the ticket: host validation, the unusual forms
http:fooandhttp:/x, and schemes without://such astag:.Tests
BoundedUrlCharSequenceNormalizerTest, 105 cases: the WPT-style URLs from the ticket removed in full, other schemes untouched, left boundaries, trailing punctuation and brackets, every kind of body delimiter (space, tab, newline, no-break space, ideographic space, U+2028, U+0085, control characters, an unpaired surrogate on either side, typographic quotes), text without a URL, URLs at the start, the end, twice, and nested, email addresses next to and inside URLs, null, empty, any CharSequence, and that deserialization yields the shared instance.UrlCharSequenceNormalizerTestgained 16 rows that pin the default's output on the same inputs. Six more cases feed a URL followed by 1000 to 5000 closing brackets through aCharSequencethat counts character reads, so a return to quadratic trimming fails the test.The WPT data (
url/resources/urltestdata.json) is not vendored: a copy would need a LICENSE and NOTICE entry for its 3-clause BSD license, which is left to the maintainers; the tests hold a curated subset instead.opennlp-runtime 2680 and opennlp-tools 546 tests with checkstyle,
-Dopennlp.forkCount=1.Manual
langdetect.xml: the normalizer row states what the default does with a port or a non-ASCII host and names the variant, and a new section "Selecting URL normalization" describes the variant's rules and how to opt in through a customLanguageDetectorFactory.Before merge
OPENNLP-1946