You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by GitBox <gi...@apache.org> on 2023/01/19 13:08:07 UTC
[GitHub] [opennlp-sandbox] mawiesne opened a new pull request, #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
mawiesne opened a new pull request, #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58
- adjusts `opennlp-tools` to 2.1.0
- adjusts parent project (org.apache.apache) to version `18`
- adjusts Java language level to 11
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] mawiesne commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082235783
##########
opennlp-coref/src/main/java/opennlp/tools/coref/resolver/AbstractResolver.java:
##########
@@ -169,7 +170,14 @@ public DiscourseEntity retain(MentionContext mention, DiscourseModel dm) {
DiscourseEntity cde = dm.getEntity(ei);
MentionContext cec = cde.getLastExtent(); // candidate extent context
if (cec.getId() == mention.getId()) {
- distances.add(ei);
+ // adding counts
+ Integer count = distances.get(ei);
Review Comment:
Will check.
##########
opennlp-coref/src/main/java/opennlp/tools/coref/resolver/MaxentResolver.java:
##########
@@ -286,7 +294,14 @@ public DiscourseEntity retain(MentionContext mention, DiscourseModel dm) {
events.add(new Event(SAME, features.toArray(new String[features.size()])));
de = cde;
//System.err.println("MaxentResolver.retain: resolved at "+ei);
- distances.add(ei);
+ // adding counts
+ Integer count = distances.get(ei);
Review Comment:
Will check.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] rzo1 commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082237964
##########
opennlp-coref/pom.xml:
##########
@@ -25,20 +25,20 @@
<parent>
<groupId>org.apache</groupId>
<artifactId>apache</artifactId>
- <version>13</version>
+ <version>18</version>
Review Comment:
https://issues.apache.org/jira/browse/OPENNLP-1452
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] mawiesne commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082245585
##########
opennlp-coref/src/main/java/opennlp/tools/coref/resolver/AbstractResolver.java:
##########
@@ -169,7 +170,14 @@ public DiscourseEntity retain(MentionContext mention, DiscourseModel dm) {
DiscourseEntity cde = dm.getEntity(ei);
MentionContext cec = cde.getLastExtent(); // candidate extent context
if (cec.getId() == mention.getId()) {
- distances.add(ei);
+ // adding counts
+ Integer count = distances.get(ei);
Review Comment:
`e` -> `ei`
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] mawiesne commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1081489081
##########
opennlp-coref/src/main/java/opennlp/tools/formats/CorefSampleStreamFactory.java:
##########
@@ -42,16 +44,19 @@ public static void registerFactory() {
StreamFactoryRegistry.registerFactory(CorefSample.class,
StreamFactoryRegistry.DEFAULT_FORMAT, new CorefSampleStreamFactory());
}
-
+
+ @Override
public ObjectStream<CorefSample> create(String[] args) {
Parameters params = ArgumentParser.parse(args, Parameters.class);
CmdLineUtil.checkInputFile("Data", params.getData());
- FileInputStream sampleDataIn = CmdLineUtil.openInFile(params.getData());
-
- ObjectStream<String> lineStream = new ParagraphStream(new PlainTextByLineStream(sampleDataIn
- .getChannel(), params.getEncoding()));
-
- return new CorefSampleDataStream(lineStream);
+ try {
+ MarkableFileInputStreamFactory factory = new MarkableFileInputStreamFactory(params.getData());
+ ObjectStream<String> lineStream = new ParagraphStream(new PlainTextByLineStream(
+ factory, params.getEncoding()));
+ return new CorefSampleDataStream(lineStream);
Review Comment:
Fixed.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] jzonthemtn merged pull request #58: Update sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
jzonthemtn merged PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] kinow commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1081436022
##########
opennlp-coref/src/main/java/opennlp/tools/cmdline/coref/CoreferencerTool.java:
##########
@@ -85,8 +85,7 @@ private void show(Parse p) {
System.out.print(" ");
}
Parse[] children = p.getChildren();
- for (int pi = 0, pn = children.length; pi < pn;pi++) {
- Parse c = children[pi];
+ for (Parse c : children) {
Review Comment:
:ok_man:
##########
opennlp-coref/src/main/java/opennlp/tools/formats/CorefSampleStreamFactory.java:
##########
@@ -42,16 +44,19 @@ public static void registerFactory() {
StreamFactoryRegistry.registerFactory(CorefSample.class,
StreamFactoryRegistry.DEFAULT_FORMAT, new CorefSampleStreamFactory());
}
-
+
+ @Override
public ObjectStream<CorefSample> create(String[] args) {
Parameters params = ArgumentParser.parse(args, Parameters.class);
CmdLineUtil.checkInputFile("Data", params.getData());
- FileInputStream sampleDataIn = CmdLineUtil.openInFile(params.getData());
-
- ObjectStream<String> lineStream = new ParagraphStream(new PlainTextByLineStream(sampleDataIn
- .getChannel(), params.getEncoding()));
-
- return new CorefSampleDataStream(lineStream);
+ try {
+ MarkableFileInputStreamFactory factory = new MarkableFileInputStreamFactory(params.getData());
+ ObjectStream<String> lineStream = new ParagraphStream(new PlainTextByLineStream(
+ factory, params.getEncoding()));
+ return new CorefSampleDataStream(lineStream);
Review Comment:
Ditto what I said in the other PR, if it makes sense, these factory and stream could be in the `try-with-resources`, assuming they are closeables and it makes sense :+1:
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] rzo1 commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082222656
##########
opennlp-coref/pom.xml:
##########
@@ -25,20 +25,20 @@
<parent>
<groupId>org.apache</groupId>
<artifactId>apache</artifactId>
- <version>13</version>
+ <version>18</version>
Review Comment:
29?
##########
opennlp-coref/src/main/java/opennlp/tools/coref/resolver/AbstractResolver.java:
##########
@@ -169,7 +170,14 @@ public DiscourseEntity retain(MentionContext mention, DiscourseModel dm) {
DiscourseEntity cde = dm.getEntity(ei);
MentionContext cec = cde.getLastExtent(); // candidate extent context
if (cec.getId() == mention.getId()) {
- distances.add(ei);
+ // adding counts
+ Integer count = distances.get(ei);
Review Comment:
`distances.merge(e, 1, Integer::sum)` ?
##########
opennlp-coref/src/main/java/opennlp/tools/coref/resolver/MaxentResolver.java:
##########
@@ -286,7 +294,14 @@ public DiscourseEntity retain(MentionContext mention, DiscourseModel dm) {
events.add(new Event(SAME, features.toArray(new String[features.size()])));
de = cde;
//System.err.println("MaxentResolver.retain: resolved at "+ei);
- distances.add(ei);
+ // adding counts
+ Integer count = distances.get(ei);
Review Comment:
`distances.merge(e, 1, Integer::sum)` ?
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] mawiesne commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082235308
##########
opennlp-coref/pom.xml:
##########
@@ -25,20 +25,20 @@
<parent>
<groupId>org.apache</groupId>
<artifactId>apache</artifactId>
- <version>13</version>
+ <version>18</version>
Review Comment:
Not yet, future work. See other comment in other PR.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp-sandbox] mawiesne commented on a diff in pull request #58: Updates sandbox component 'opennlp-coref' to be compatible with latest opennlp-tools release
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #58:
URL: https://github.com/apache/opennlp-sandbox/pull/58#discussion_r1082250243
##########
opennlp-coref/pom.xml:
##########
@@ -25,20 +25,20 @@
<parent>
<groupId>org.apache</groupId>
<artifactId>apache</artifactId>
- <version>13</version>
+ <version>18</version>
Review Comment:
I'll leave a TODO referencing this issue number here.
--
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: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org