You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/04/09 10:01:41 UTC
[GitHub] [commons-text] bradleyrumball commented on a change in pull request #214: Fixed JaroWinklerDistance
bradleyrumball commented on a change in pull request #214:
URL: https://github.com/apache/commons-text/pull/214#discussion_r610499047
##########
File path: src/main/java/org/apache/commons/text/similarity/JaroWinklerDistance.java
##########
@@ -71,20 +76,7 @@ public Double apply(final CharSequence left, final CharSequence right) {
throw new IllegalArgumentException("CharSequences must not be null");
}
- // TODO: replace the rest of the code by this in 2.0, see TEXT-104
- //
- // JaroWinklerSimilarity similarity = new JaroWinklerSimilarity();
- // return 1 - similarity.apply(left, right);
-
- final double defaultScalingFactor = 0.1;
- final int[] mtp = matches(left, right);
- final double m = mtp[0];
- if (m == 0) {
- return 0D;
- }
- final double j = ((m / left.length() + m / right.length() + (m - (double) mtp[1] / 2) / m)) / 3;
- final double jw = j < 0.7D ? j : j + defaultScalingFactor * mtp[2] * (1D - j);
- return jw;
+ return 1 - similarity.apply(left, right);
Review comment:
Hi Kinow!
I wanted to do this as it seemed a far superior approach but wasn't sure if the repetition was present to limit stack depth.
This looks good and is far superior.
##########
File path: src/test/java/org/apache/commons/text/similarity/JaroWinklerDistanceTest.java
##########
@@ -36,34 +36,22 @@ public static void setUp() {
@Test
public void testGetJaroWinklerDistance_StringString() {
- assertEquals(0.92499d, distance.apply("frog", "fog"), 0.00001d);
- assertEquals(0.0d, distance.apply("fly", "ant"), 0.00000000000000000001d);
- assertEquals(0.44166d, distance.apply("elephant", "hippo"), 0.00001d);
- assertEquals(0.90666d, distance.apply("ABC Corporation", "ABC Corp"), 0.00001d);
- assertEquals(0.95251d, distance.apply("D N H Enterprises Inc", "D & H Enterprises, Inc."), 0.00001d);
- assertEquals(0.942d, distance.apply("My Gym Children's Fitness Center", "My Gym. Childrens Fitness"), 0.00001d);
- assertEquals(0.898018d, distance.apply("PENNSYLVANIA", "PENNCISYLVNIA"), 0.00001d);
- assertEquals(0.971428d, distance.apply("/opt/software1", "/opt/software2"), 0.00001d);
- assertEquals(0.941666d, distance.apply("aaabcd", "aaacdb"), 0.00001d);
- assertEquals(0.911111d, distance.apply("John Horn", "John Hopkins"), 0.00001d);
- // TODO: replace tests in 2.0. See TEXT-104 for more.
- // assertEquals(0d, distance.apply("", ""), 0.00001d);
- // assertEquals(0d, distance.apply("foo", "foo"), 0.00001d);
- // assertEquals(1 - 0.94166d, distance.apply("foo", "foo "), 0.00001d);
- // assertEquals(1 - 0.90666d, distance.apply("foo", "foo "), 0.00001d);
- // assertEquals(1 - 0.86666d, distance.apply("foo", " foo "), 0.00001d);
- // assertEquals(1 - 0.51111d, distance.apply("foo", " foo"), 0.00001d);
- // assertEquals(1 - 0.92499d, distance.apply("frog", "fog"), 0.00001d);
- // assertEquals(1.0d, distance.apply("fly", "ant"), 0.00000000000000000001d);
- // assertEquals(1 - 0.44166d, distance.apply("elephant", "hippo"), 0.00001d);
- // assertEquals(1 - 0.90666d, distance.apply("ABC Corporation", "ABC Corp"), 0.00001d);
- // assertEquals(1 - 0.95251d, distance.apply("D N H Enterprises Inc", "D & H Enterprises, Inc."), 0.00001d);
- // assertEquals(1 - 0.942d,
- // distance.apply("My Gym Children's Fitness Center", "My Gym. Childrens Fitness"), 0.00001d);
- // assertEquals(1 - 0.898018d, distance.apply("PENNSYLVANIA", "PENNCISYLVNIA"), 0.00001d);
- // assertEquals(1 - 0.971428d, distance.apply("/opt/software1", "/opt/software2"), 0.00001d);
- // assertEquals(1 - 0.941666d, distance.apply("aaabcd", "aaacdb"), 0.00001d);
- // assertEquals(1 - 0.911111d, distance.apply("John Horn", "John Hopkins"), 0.00001d);
+ assertEquals(0.07501d, distance.apply("frog", "fog"), 0.00001d);
+ assertEquals(1.0d, distance.apply("fly", "ant"), 0.00000000000000000001d);
+ assertEquals(0.55834d, distance.apply("elephant", "hippo"), 0.00001d);
+ assertEquals(0.09334d, distance.apply("ABC Corporation", "ABC Corp"), 0.00001d);
+ assertEquals(0.04749d, distance.apply("D N H Enterprises Inc", "D & H Enterprises, Inc."), 0.00001d);
+ assertEquals(0.058d, distance.apply("My Gym Children's Fitness Center", "My Gym. Childrens Fitness"), 0.00001d);
+ assertEquals(0.101982d, distance.apply("PENNSYLVANIA", "PENNCISYLVNIA"), 0.00001d);
+ assertEquals(0.028572d, distance.apply("/opt/software1", "/opt/software2"), 0.00001d);
+ assertEquals(0.058334d, distance.apply("aaabcd", "aaacdb"), 0.00001d);
+ assertEquals(0.088889d, distance.apply("John Horn", "John Hopkins"), 0.00001d);
+ assertEquals(0d, distance.apply("", ""), 0.00001d);
+ assertEquals(0d, distance.apply("foo", "foo"), 0.00001d);
+ assertEquals(1 - 0.94166d, distance.apply("foo", "foo "), 0.00001d);
+ assertEquals(1 - 0.90666d, distance.apply("foo", "foo "), 0.00001d);
+ assertEquals(1 - 0.86666d, distance.apply("foo", " foo "), 0.00001d);
+ assertEquals(1 - 0.51111d, distance.apply("foo", " foo"), 0.00001d);
Review comment:
Looks good; the only thing I can note is the (1 - double) in the 4 new cases if we wanted to be consistent with the other tests. Otherwise, happy days!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org