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 2022/12/11 11:26:50 UTC
[GitHub] [opennlp] mawiesne opened a new pull request, #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
mawiesne opened a new pull request, #456:
URL: https://github.com/apache/opennlp/pull/456
Change
-
- adds missing JavaDoc
- improves existing documentation for clarity
- removes superfluous text
- adds 'final' modifier where useful and applicable
- adds 'Override' annotation where useful and applicable
- adds constructor `BaseModel(String componentName, Path modelPath)` helps in extending classes to use `Path` instances more intuitively; see: `TokenNameFinderModel(Path modelPath)`
- fixes several typos
- cures an undetected runtime error in `TokenNameFinderFactory#instantiateSequenceCodec()` which failed to handle a potential `InvalidFormatException`. Please, check and discuss my **TODO** in `TokenNameFinderFactory#createSequenceCodec()` on the compensation strategy!
Tasks
-
Thank you for contributing to Apache OpenNLP.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [x] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp folder?
### For documentation related changes:
- [x] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions for build issues and submit an update to your PR as soon as possible.
--
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] atarora commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
atarora commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052160001
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderModel.java:
##########
@@ -88,37 +117,109 @@ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, in
}
}
- // TODO: Extend this one with beam size!
- public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel,
- byte[] generatorDescriptor, Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, byte[] generatorDescriptor,
+ Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
this(languageCode, nameFinderModel, NameFinderME.DEFAULT_BEAM_SIZE,
- generatorDescriptor, resources, manifestInfoEntries, new BioCodec(), new TokenNameFinderFactory());
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
}
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param beamSize The beam size. Must be greater than {@code 0}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, int beamSize,
+ byte[] generatorDescriptor, Map<String, Object> resources,
+ Map<String, String> manifestInfoEntries) {
+ this(languageCode, nameFinderModel, beamSize,
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
+ }
+
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
Review Comment:
namFinderModel - nameFinderModel ?
--
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] kinow commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052237129
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
+1 to A or B (we've made a few binary backward incompatible changes anyway :shrug: )
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
+1 to A or B (we've made a few binary backward incompatible changes anyway )
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052254776
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
Please @rzo1 share your thoughts on (A) - (D) and leave a thumb or separate comment.
--
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] rzo1 commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1046207440
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -133,18 +173,36 @@ public void validateArtifactMap() throws InvalidFormatException {
// no additional artifacts
}
+ /**
+ * @return Retrieves a {@link SequenceCodec} instance.
+ */
public SequenceCodec<String> createSequenceCodec() {
if (artifactProvider != null) {
- String sequeceCodecImplName = artifactProvider.getManifestProperty(
+ String sequenceCodecImplName = artifactProvider.getManifestProperty(
TokenNameFinderModel.SEQUENCE_CODEC_CLASS_NAME_PARAMETER);
- return instantiateSequenceCodec(sequeceCodecImplName);
+ try {
+ return instantiateSequenceCodec(sequenceCodecImplName);
+ } catch (InvalidFormatException e) {
+ // TODO Is this compensation valid here? Or: should we escalate towards the caller?
+ System.out.println("Encountered a problem with the sequence codec '"
Review Comment:
Yes - logging as a topic might be something to discuss on the dev@ list outlining different approaches / possibilities like using a bridge (like slf4j-api) and let the user add the log impl, rely on JUL (no 3rd party deps) and let users add bridges, etc. - can think of some more :)
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052221134
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderModel.java:
##########
@@ -88,37 +117,109 @@ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, in
}
}
- // TODO: Extend this one with beam size!
- public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel,
- byte[] generatorDescriptor, Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, byte[] generatorDescriptor,
+ Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
this(languageCode, nameFinderModel, NameFinderME.DEFAULT_BEAM_SIZE,
- generatorDescriptor, resources, manifestInfoEntries, new BioCodec(), new TokenNameFinderFactory());
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
}
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param beamSize The beam size. Must be greater than {@code 0}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, int beamSize,
+ byte[] generatorDescriptor, Map<String, Object> resources,
+ Map<String, String> manifestInfoEntries) {
+ this(languageCode, nameFinderModel, beamSize,
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
+ }
+
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
Review Comment:
Will also fix the missing "is" 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
[GitHub] [opennlp] jzonthemtn commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
jzonthemtn commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1045808271
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -87,24 +104,49 @@ private static byte[] loadDefaultFeatureGeneratorBytes() {
}
}
catch (IOException e) {
- throw new IllegalStateException("Failed reading from ner-default-features.xml file on classpath!");
+ throw new IllegalStateException("Failed reading from 'ner-default-features.xml' file on classpath!");
}
return bytes.toByteArray();
}
+ /**
+ * @return Retrieves the {@link SequenceCodec} in use.
+ */
protected SequenceCodec<String> getSequenceCodec() {
return seqCodec;
}
+ /**
+ * @return Retrieves the additional {@code resources} in use.
+ */
protected Map<String, Object> getResources() {
return resources;
}
+ /**
+ * @return Retrieves {@code byte[]} in use representing the feature generator descriptor.
+ */
protected byte[] getFeatureGenerator() {
return featureGeneratorBytes;
}
+
+ /**
+ * Initializes a {@link TokenNameFinderFactory} instance via given parameters.
+ *
+ * @param subclassName The class name used for instantiation. If {@code null}, an
+ * instance of {@link TokenNameFinderFactory} will be returned
+ * per default. Otherwise, the {@link ExtensionLoader} mechanism
+ * is applied to load the requested {@code subclassName}.
+ * @param featureGeneratorBytes The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param seqCodec The {@link SequenceCodec} to use.
+ *
+ * @return A valid {@link TokenNameFinderFactory} instance.
+ * @throws InvalidFormatException Thrown if the {@link ExtensionLoader} mechanism failed to
+ * create the factory associated with {@code subclassName}.
+ */
public static TokenNameFinderFactory create(String subclassName, byte[] featureGeneratorBytes,
final Map<String, Object> resources, SequenceCodec<String> seqCodec)
throws InvalidFormatException {
Review Comment:
Can it still throw an `InvalidFormatException`?
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052216345
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderModel.java:
##########
@@ -88,37 +117,109 @@ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, in
}
}
- // TODO: Extend this one with beam size!
- public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel,
- byte[] generatorDescriptor, Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, byte[] generatorDescriptor,
+ Map<String, Object> resources, Map<String, String> manifestInfoEntries) {
this(languageCode, nameFinderModel, NameFinderME.DEFAULT_BEAM_SIZE,
- generatorDescriptor, resources, manifestInfoEntries, new BioCodec(), new TokenNameFinderFactory());
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
}
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param beamSize The beam size. Must be greater than {@code 0}.
+ * @param generatorDescriptor The {@code byte[]} representing the feature generator descriptor.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
+ * with {@code seqCodec}.
+ */
+ public TokenNameFinderModel(String languageCode, MaxentModel nameFinderModel, int beamSize,
+ byte[] generatorDescriptor, Map<String, Object> resources,
+ Map<String, String> manifestInfoEntries) {
+ this(languageCode, nameFinderModel, beamSize,
+ generatorDescriptor, resources, manifestInfoEntries,
+ new BioCodec(), new TokenNameFinderFactory());
+ }
+
+ /**
+ * Initializes a {@link TokenNameFinderModel} instance via given parameters.
+ *
+ * @param languageCode The ISO conform language code.
+ * @param nameFinderModel A valid {@link MaxentModel}.
+ * @param resources Additional resources in a mapping.
+ * @param manifestInfoEntries Additional information kept in the manifest.
+ *
+ * @throws IllegalArgumentException Thrown if the {@code namFinderModel} incompatible
Review Comment:
Eagle eye!
--
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] kinow commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1045223051
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/BioCodec.java:
##########
@@ -26,23 +26,39 @@
import opennlp.tools.util.SequenceCodec;
import opennlp.tools.util.Span;
+/**
+ * The default {@link SequenceCodec} implementation according to the {@code BIO} scheme:
+ * <ul>
+ * <li>B: 'beginning' of a NE)</li>
+ * <li>I: 'inside', the word is inside a NE</li>
+ * <li>O: 'outside', the word is a regular word outside a NE)</li>
Review Comment:
Same here?
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/BilouCodec.java:
##########
@@ -27,6 +27,23 @@
import opennlp.tools.util.SequenceValidator;
import opennlp.tools.util.Span;
+/**
+ * The default {@link SequenceCodec} implementation according to the {@code BILOU} scheme.
+ * <ul>
+ * <li>B: 'beginning' of a NE)</li>
+ * <li>I: 'inside', the word is inside a NE)</li>
Review Comment:
I think the `NE)` has an extra `)` in these two cases?
--
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] kinow commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052046193
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/NameFinderEventStream.java:
##########
@@ -32,36 +32,38 @@
import opennlp.tools.util.featuregen.WindowFeatureGenerator;
/**
- * Class for creating an event stream out of data files for training an name
- * finder.
+ * Class for creating an event stream out of data files for training an {@link TokenNameFinder}.
*/
public class NameFinderEventStream extends opennlp.tools.util.AbstractEventStream<NameSample> {
- private NameContextGenerator contextGenerator;
+ private final NameContextGenerator contextGenerator;
- private AdditionalContextFeatureGenerator additionalContextFeatureGenerator =
+ private final AdditionalContextFeatureGenerator additionalContextFeatureGenerator =
new AdditionalContextFeatureGenerator();
- private SequenceCodec<String> codec;
+ private final SequenceCodec<String> codec;
private final String defaultType;
/**
- * Creates a new name finder event stream using the specified data stream and context generator.
- * @param dataStream The data stream of events.
- * @param type null or overrides the type parameter in the provided samples
- * @param contextGenerator The context generator used to generate features for the event stream.
+ * Initializes a {@link NameFinderEventStream} using the specified {@code dataStream} and
+ * {@link NameContextGenerator}.
+ *
+ * @param dataStream The {@link ObjectStream data stream} of events.
+ * @param type {@code null} or overrides the type parameter in the provided samples.
+ * @param contextGenerator The {@link NameContextGenerator} used to generate features for the event stream.
+ * @param codec The {@link SequenceCodec} to use.
*/
public NameFinderEventStream(ObjectStream<NameSample> dataStream, String type,
NameContextGenerator contextGenerator, SequenceCodec<String> codec) {
super(dataStream);
- this.codec = codec;
-
if (codec == null) {
this.codec = new BioCodec();
+ } else {
+ this.codec = codec;
}
-
+
Review Comment:
Extra spaces
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
Now this one got me... strings/input? Probably better refactor or remove.
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
Matches? Or a typo...
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052564888
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
Option A it is, will proceed with that.
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
Option A it is, will proceed with that.
--
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] jzonthemtn commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
jzonthemtn commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1045808696
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -118,9 +160,7 @@ public static TokenNameFinderFactory create(String subclassName, byte[] featureG
TokenNameFinderFactory.class, subclassName);
} catch (Exception e) {
String msg = "Could not instantiate the " + subclassName
- + ". The initialization throw an exception.";
- System.err.println(msg);
- e.printStackTrace();
+ + ". The initialization threw an exception.";
throw new InvalidFormatException(msg, e);
Review Comment:
I see - it can still throw `InvalidFormatException` 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
[GitHub] [opennlp] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052218809
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/NameFinderEventStream.java:
##########
@@ -32,36 +32,38 @@
import opennlp.tools.util.featuregen.WindowFeatureGenerator;
/**
- * Class for creating an event stream out of data files for training an name
- * finder.
+ * Class for creating an event stream out of data files for training an {@link TokenNameFinder}.
*/
public class NameFinderEventStream extends opennlp.tools.util.AbstractEventStream<NameSample> {
- private NameContextGenerator contextGenerator;
+ private final NameContextGenerator contextGenerator;
- private AdditionalContextFeatureGenerator additionalContextFeatureGenerator =
+ private final AdditionalContextFeatureGenerator additionalContextFeatureGenerator =
new AdditionalContextFeatureGenerator();
- private SequenceCodec<String> codec;
+ private final SequenceCodec<String> codec;
private final String defaultType;
/**
- * Creates a new name finder event stream using the specified data stream and context generator.
- * @param dataStream The data stream of events.
- * @param type null or overrides the type parameter in the provided samples
- * @param contextGenerator The context generator used to generate features for the event stream.
+ * Initializes a {@link NameFinderEventStream} using the specified {@code dataStream} and
+ * {@link NameContextGenerator}.
+ *
+ * @param dataStream The {@link ObjectStream data stream} of events.
+ * @param type {@code null} or overrides the type parameter in the provided samples.
+ * @param contextGenerator The {@link NameContextGenerator} used to generate features for the event stream.
+ * @param codec The {@link SequenceCodec} to use.
*/
public NameFinderEventStream(ObjectStream<NameSample> dataStream, String type,
NameContextGenerator contextGenerator, SequenceCodec<String> codec) {
super(dataStream);
- this.codec = codec;
-
if (codec == null) {
this.codec = new BioCodec();
+ } else {
+ this.codec = codec;
}
-
+
Review Comment:
will fix.
--
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] rzo1 commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052554159
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
A or B
--
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] mawiesne commented on pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on PR #456:
URL: https://github.com/apache/opennlp/pull/456#issuecomment-1361001816
@jzonthemtn Feedback and review comments have all been addressed with this PR. I'd be happy if you review and/or approve the PR from your side.
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1048939494
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -133,18 +173,36 @@ public void validateArtifactMap() throws InvalidFormatException {
// no additional artifacts
}
+ /**
+ * @return Retrieves a {@link SequenceCodec} instance.
+ */
public SequenceCodec<String> createSequenceCodec() {
if (artifactProvider != null) {
- String sequeceCodecImplName = artifactProvider.getManifestProperty(
+ String sequenceCodecImplName = artifactProvider.getManifestProperty(
TokenNameFinderModel.SEQUENCE_CODEC_CLASS_NAME_PARAMETER);
- return instantiateSequenceCodec(sequeceCodecImplName);
+ try {
+ return instantiateSequenceCodec(sequenceCodecImplName);
+ } catch (InvalidFormatException e) {
+ // TODO Is this compensation valid here? Or: should we escalate towards the caller?
+ System.out.println("Encountered a problem with the sequence codec '"
Review Comment:
@jzonthemtn I'll remove the sysout here tmrw. The discussion about logger or not seems a separate issue and/or a discussion for the dev lis, that is: can't be solved with this 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] atarora commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
atarora commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052157920
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
This looks like type of opennlp.tools.util.Span.class
--
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] mawiesne merged pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne merged PR #456:
URL: https://github.com/apache/opennlp/pull/456
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052226452
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
@kinow @atarora What's your opinion in terms of
"refactoring, deprecation, or removal" of this unused method?
Either
- (A) refactor it to `getSpanType()`
- (B) refactor it to `getSpanType()` and deprecate
- (C) leave method name unchanged and provide an explanation of "s" via JavaDoc text
- (D) remove without deprecation period
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052254418
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
Please @rzo1 share your thoughts on (A) - (D) and leave a thumb or separate comment.
--
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] jzonthemtn commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
jzonthemtn commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1046184927
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -133,18 +173,36 @@ public void validateArtifactMap() throws InvalidFormatException {
// no additional artifacts
}
+ /**
+ * @return Retrieves a {@link SequenceCodec} instance.
+ */
public SequenceCodec<String> createSequenceCodec() {
if (artifactProvider != null) {
- String sequeceCodecImplName = artifactProvider.getManifestProperty(
+ String sequenceCodecImplName = artifactProvider.getManifestProperty(
TokenNameFinderModel.SEQUENCE_CODEC_CLASS_NAME_PARAMETER);
- return instantiateSequenceCodec(sequeceCodecImplName);
+ try {
+ return instantiateSequenceCodec(sequenceCodecImplName);
+ } catch (InvalidFormatException e) {
+ // TODO Is this compensation valid here? Or: should we escalate towards the caller?
+ System.out.println("Encountered a problem with the sequence codec '"
Review Comment:
Regarding the logging, the project has always tried to maintain a principle of using the fewest dependencies as possible. The log4j vulnerability a few months ago is a good example why. That can certainly be brought up again for discussion on the mailing list, but a better logging solution that doesn't require a new dependency would be ideal (if it even exists).
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1045225673
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/BilouCodec.java:
##########
@@ -27,6 +27,23 @@
import opennlp.tools.util.SequenceValidator;
import opennlp.tools.util.Span;
+/**
+ * The default {@link SequenceCodec} implementation according to the {@code BILOU} scheme.
+ * <ul>
+ * <li>B: 'beginning' of a NE)</li>
+ * <li>I: 'inside', the word is inside a NE)</li>
Review Comment:
Will fix.
--
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] rzo1 commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1045950983
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/TokenNameFinderFactory.java:
##########
@@ -133,18 +173,36 @@ public void validateArtifactMap() throws InvalidFormatException {
// no additional artifacts
}
+ /**
+ * @return Retrieves a {@link SequenceCodec} instance.
+ */
public SequenceCodec<String> createSequenceCodec() {
if (artifactProvider != null) {
- String sequeceCodecImplName = artifactProvider.getManifestProperty(
+ String sequenceCodecImplName = artifactProvider.getManifestProperty(
TokenNameFinderModel.SEQUENCE_CODEC_CLASS_NAME_PARAMETER);
- return instantiateSequenceCodec(sequeceCodecImplName);
+ try {
+ return instantiateSequenceCodec(sequenceCodecImplName);
+ } catch (InvalidFormatException e) {
+ // TODO Is this compensation valid here? Or: should we escalate towards the caller?
+ System.out.println("Encountered a problem with the sequence codec '"
Review Comment:
I don't know, if it is natural, that we return a `BioCodec` instance or something else from `init(...)` in case we fail to load the requested codec from the given `ArtifactProvider`. My expectation - as a user - would be, that I get an exception, which tells me, that my `codec`, which I specified via the manifest property couldn't be loaded via the given artifact loader and I can deal with that in my app.
Regarding the second half of @jzonthemtn comment:
We should replace `System.out.println` and `System.err.println` with calls to some logging library any time soon. These calls might be thrown into the nether, if OpenNLP is used within an application server for example.
--
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] atarora commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
atarora commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052153428
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
I believe it refers to the pattern (java.util.regex.Pattern.class) of the matched value. (ref : RegexNameFinder.java -> L92)
--
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] mawiesne commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
mawiesne commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052225879
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
@kinow @atarora What's your opinion in terms of
"refactoring, deprecation, or removal" of this unused method?
Either
- (A) refactor it to `getMatchingPatterns()`
- (B) refactor it to `getMatchingPatterns()` and deprecate
- (C) leave method name unchanged and provide an explanation of "m" via JavaDoc text
- (D) remove without deprecation period
--
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] rzo1 commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
rzo1 commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052554362
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
return mPatterns;
}
+ /**
+ * @param mPatterns The {@link Pattern patterns} to be set.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public void setmPatterns(Pattern[] mPatterns) {
this.mPatterns = mPatterns;
}
+ /**
+ * @return Retrieves the type used.
+ */
+ // TODO Unclear what 's' stands for: Discuss refactoring, deprecation, or removal of unused method.
public String getsType() {
Review Comment:
A or B
--
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] kinow commented on a diff in pull request #456: OPENNLP-1410 Enhance JavaDoc in opennlp.tools.namefind package
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #456:
URL: https://github.com/apache/opennlp/pull/456#discussion_r1052237462
##########
opennlp-tools/src/main/java/opennlp/tools/namefind/RegexNameFinder.java:
##########
@@ -178,18 +197,34 @@ public void clearAdaptiveData() {
// nothing to clear
}
+ /**
+ * @return Retrieves the {@link Pattern patterns} used.
+ */
+ // TODO Unclear what 'm' stands for: Discuss refactoring, deprecation, or removal of unused method.
public Pattern[] getmPatterns() {
Review Comment:
+1 to A or B
--
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