kinow commented on a change in pull request #214:
URL: https://github.com/apache/commons-text/pull/214#discussion_r610360084
##########
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:
I deleted the commented lines, but moved these 4 cases that were not in
your original PR @bradleyrumball .
##########
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:
I used the `JaroWinklerSimilarity` as mentioned in the JIRA issue.
In TEXT-104, we had a regression as I had removed the `matches` method, that
is now deprecated. When I reverted it, I could have also fixed the behaviour
here.
Even though this is a change that could break the code of existing users,
it's actually fixing a bug in the way we calculate the edit distance, making
similarity and distance return different values, and the distance will be `0`
for identical values, and `1` for the most distant value possible — as with
other distances.
I used some Python libraries to compare the output, and the value of our
`similarity` matches their similarity/metric/or distance.
--
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:
us...@infra.apache.org
