krickert commented on PR #1482: URL: https://github.com/apache/opennlp/pull/1482#issuecomment-6111919014
> Hi @krickert - thanks for the work here. Playing devil's advocate: I'm not sure we need this PR. > > * Head-rule tags are user-supplied regular expressions, which is exactly the case #1348 allows with `@SuppressForbidden`. The guard already passes with that suppression, and the escaping bug in the bundled rules is fixed by #1461. > * The new rule format (`# opennlp-ancora-head-rules 2`) is stored in trained models, so Spanish parser models trained with 3.0 would no longer load in 2.x. > * Converting legacy rule files changes how existing models pick heads (`GRUP\\.VERB` suddenly matches), so models trained before 3.0 would need retraining. #1461 deliberately avoided that. > * Custom rule files that use other regex constructs would stop loading. > * No measured performance or correctness problem is addressed, and we would maintain a small regex engine of our own instead of `java.util.regex`. > > I'd suggest closing this, or deferring it beyond 3.0.0. If the clearer load errors are the main benefit, we could add line-numbered validation on top of the existing regex matching, without a new format version. WDYT? I'll close it. I was looking under covers and looking for any regex that might come out of the woodwork and found 2 PRs, this is one of them. Let's see how the other one does... -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
