[PR] Minor: Share layer alignment and span mapping across document annotators (opennlp)

["rzo1 (via GitHub)" <gi...@apache.org>]

rzo1 opened a new pull request, #1357:
URL: https://github.com/apache/opennlp/pull/1357

   Adds requireAligned, values and toCharacterSpan to DocumentAnnotators and uses them in the chunker, lemmatizer and name finder annotators instead of their own alignment checks, tag gathering and span mapping. TermVectorAnnotator now uses requireLayers. Behavior is unchanged.


-- 
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 layer alignment and span mapping across document annotators (opennlp)

["krickert (via GitHub)" <gi...@apache.org>]

krickert commented on PR #1357:
URL: https://github.com/apache/opennlp/pull/1357#issuecomment-5957354159

   Made a minor edit:
   
   The alignment check only compared layer sizes, so a document missing a layer slipped through or got the wrong error. Now it'll first check that the layers exist, and then the parser annotator uses the shared span helper like the other three annotators instead of its own copy.
   
   AI summary (+ my edits - the lower level details):
   
   > Copied the pre-PR inline span mapping and tag gather from `ChunkerAnnotator` and `NameFinderAnnotator` and ran them next to the new `DocumentAnnotators` over a token layer with a combining mark and a supplementary character, for all (start, end) combinations from -2 to count+2: 100 cases, 0 differences, same exception type and text. `requireAligned` and `requireLayers` produce the exact messages the removed inline checks produced. The existing message assertions in `NameFinderAnnotatorTest` and `LemmatizerAnnotatorTest` still pass. `api` and `runtime` pass locally.
   
   I pushed a red test and a fix on top:
   
   - `requireAligned` documents that the document must have both layers but only compared sizes. `Document.get` returns an empty list for a missing layer, so one missing layer was reported as "misaligned" and a document with no layers at all passed. It now calls `requireLayers` first. The annotators don't change, they all call `requireLayers` before it.
   - `ParserAnnotator` still built the phrase span itself from the token layer. It uses `toCharacterSpan` now like the chunker and the name finder, so the title is true for all the annotators.
   


-- 
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 layer alignment and span mapping across document annotators (opennlp)

["mawiesne (via GitHub)" <gi...@apache.org>]

mawiesne merged PR #1357:
URL: https://github.com/apache/opennlp/pull/1357


-- 
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 layer alignment and span mapping across document annotators (opennlp)

["rzo1 (via GitHub)" <gi...@apache.org>]

rzo1 commented on PR #1357:
URL: https://github.com/apache/opennlp/pull/1357#issuecomment-5959377933

   Thx for review. Updated the description.


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