OPENNLP-983: Allows length for Prefix and Suffix feature generators to be set #121
Conversation
|
Thank you for your PR. The code looks great. Can you please squash the commit and fix the first line of yours commit message? The first line should 72 characters max. |
jzonthemtn
force-pushed
the
OPENNLP-983
branch
from
4dda548 to
938fb91
Compare
|
@wcolen Sure -- I think that's got it. |
| } | ||
|
|
||
| @Test | ||
| public void lengthTest2() { |
There was a problem hiding this comment.
Nitpicking: possible to come up with a better name for this - never mind if its too long, but it would make better sense to a first timer reading this code
There was a problem hiding this comment.
Totally agree. Those test names are not the most descriptive. I'll consider their names again. If you think of anything better in the meantime let me know.
|
+1 lgtm |
| return prefs; | ||
| public static final int DEFAULT_MAX_LENGTH = 4; | ||
|
|
||
| private int prefixLength; |
| return suffs; | ||
| public static final int DEFAULT_MAX_LENGTH = 4; | ||
|
|
||
| private int suffixLength; |
|
Looks like this is almost good to be merged. Can you squash the commits? And address the remaining feedback. |
jzonthemtn
force-pushed
the
OPENNLP-983
branch
from
8fa09ad to
9d25705
Compare
|
@kottmann Yes, commits squashed, added new lines, and made the ints final. |
4 participants
This pull request makes the length for the Prefix- and SuffixFeatureGenerator configurable. It defaults to 4 (the original value) if not provided. This pull request also prevents duplicate features from being returned in cases where the given length is larger than the token's size.