This is an automated email from the ASF dual-hosted git repository. mawiesne pushed a commit to branch OPENNLP-509_opennlp.tools.parser.Parse.getParent()_returning_incorrect_object in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit 078a46fcdb33835ed463e25372ca8ce9cb8279da Author: Martin Wiesner <[email protected]> AuthorDate: Sat Mar 4 21:58:37 2023 +0100 OPENNLP-509 opennlp.tools.parser.Parse.getParent() returning incorrect object - adds `setParents(..)` calls in `AbstractBottomUpParser#parse(Parse tokens, int numParses)` which now set the hierarchical (back) references that were missing before - adds test to verify that a processed Parse instance's parent references are populated and _not_ `null` --- .../tools/parser/AbstractBottomUpParser.java | 17 +++++++- .../tools/parser/AbstractParserModelTest.java | 51 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/opennlp-tools/src/main/java/opennlp/tools/parser/AbstractBottomUpParser.java b/opennlp-tools/src/main/java/opennlp/tools/parser/AbstractBottomUpParser.java index d0454c45..9f0bcde2 100644 --- a/opennlp-tools/src/main/java/opennlp/tools/parser/AbstractBottomUpParser.java +++ b/opennlp-tools/src/main/java/opennlp/tools/parser/AbstractBottomUpParser.java @@ -338,15 +338,30 @@ public abstract class AbstractBottomUpParser implements Parser { odh = ndh; } if (completeParses.size() == 0) { + if (guess != null) { + setParents(guess); + for (Parse childGuess: guess.getChildren()) { + setParents(childGuess); + } + } return new Parse[] {guess}; } else if (numParses == 1) { - return new Parse[] {completeParses.first()}; + Parse best = completeParses.first(); + setParents(best); + for (Parse childBest: best.getChildren()) { + setParents(childBest); + } + return new Parse[] {best}; } else { List<Parse> topParses = new ArrayList<>(numParses); while (!completeParses.isEmpty() && topParses.size() < numParses) { Parse tp = completeParses.first(); + setParents(tp); + for (Parse childTp: tp.getChildren()) { + setParents(childTp); + } completeParses.remove(tp); topParses.add(tp); //parses.remove(tp); diff --git a/opennlp-tools/src/test/java/opennlp/tools/parser/AbstractParserModelTest.java b/opennlp-tools/src/test/java/opennlp/tools/parser/AbstractParserModelTest.java index 346e4eb7..7960131c 100644 --- a/opennlp-tools/src/test/java/opennlp/tools/parser/AbstractParserModelTest.java +++ b/opennlp-tools/src/test/java/opennlp/tools/parser/AbstractParserModelTest.java @@ -91,6 +91,57 @@ public abstract class AbstractParserModelTest { Assertions.assertNotNull(s); } + /* + * Verifies / addresses OPENNLP-509 + * See: https://issues.apache.org/jira/projects/OPENNLP/issues/OPENNLP-509 + */ + @Test + void testParsingCheckParentReferencesArePopulated() { + // fixtures + final String sent = "Martin is testing."; + // prepare + List<String> tokens = Arrays.asList(WhitespaceTokenizer.INSTANCE.tokenize(sent)); + String text = String.join(" ", tokens); + + Parse sentP = new Parse(text, new Span(0, text.length()), + AbstractBottomUpParser.INC_NODE, 0, null); + int start = 0; + int i = 0; + for (Iterator<String> ti = tokens.iterator(); ti.hasNext(); i++) { + String tok = ti.next(); + sentP.insert(new Parse(text, new Span(start, start + tok.length()), + AbstractBottomUpParser.TOK_NODE, 0, i)); + start += tok.length() + 1; + } + + Parser parser = ParserFactory.create(getModel()); + Assertions.assertNotNull(parser); + + // Verifies parents of top-k parses (k=2) + Parse[] parses = parser.parse(sentP, 2); + Assertions.assertNotNull(parses); + for (Parse parent : parses) { + checkParentsEqual(parent); + } + } + + /* + * Recursively traverses the parse tree and verifies parent references are populated. + */ + private void checkParentsEqual(Parse parent) { + for (Parse child : parent.getChildren()) { + Parse cParent = child.getParent(); + // System.out.println(cParent.toStringPennTreebank() " --- type: " cParent.getType()); + if (AbstractBottomUpParser.TOK_NODE.equals(child.getType())) { + return; // found a leaf node: stopping recursion + } + Assertions.assertEquals(parent, cParent); + if (cParent.getChildren() != null) { + checkParentsEqual(child); + } + } + } + /* * Verifies changes in OPENNLP-1330 and addresses follow-up OPENNLP-1333 * See: https://issues.apache.org/jira/projects/OPENNLP/issues/OPENNLP-1333
