From 2b2957d367d721f519a1f2ca8024b8fec3984ccd Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 8 Jan 2025 22:10:43 -0500 Subject: [PATCH] refactor: DH-18308: Remove NullNanAwareComparator in favor of built-in Comparators. (#6513) --- .../comparators/NullNaNAwareComparator.java | 61 ------------ engine/function/src/templates/Sort.ftl | 17 ++-- engine/function/src/templates/TestSort.ftl | 36 +++++++ .../TestNullNaNAwareComparator.java | 97 ------------------- 4 files changed, 44 insertions(+), 167 deletions(-) delete mode 100644 engine/function/src/main/java/io/deephaven/function/comparators/NullNaNAwareComparator.java delete mode 100644 engine/function/src/test/java/io/deephaven/function/comparators/TestNullNaNAwareComparator.java diff --git a/engine/function/src/main/java/io/deephaven/function/comparators/NullNaNAwareComparator.java b/engine/function/src/main/java/io/deephaven/function/comparators/NullNaNAwareComparator.java deleted file mode 100644 index 0a5e8b2d9ff..00000000000 --- a/engine/function/src/main/java/io/deephaven/function/comparators/NullNaNAwareComparator.java +++ /dev/null @@ -1,61 +0,0 @@ -// -// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending -// -package io.deephaven.function.comparators; - -import java.util.Comparator; - -import static io.deephaven.function.Basic.isNull; -import static io.deephaven.function.Numeric.isNaN; - -/** - * Comparator which is aware of NaN and Null values, in the Deephaven convention. - * - * @param type to compare. - */ -public class NullNaNAwareComparator> implements Comparator { - - /** - * Creates a comparator. - */ - public NullNaNAwareComparator() {} - - private static boolean isNanVal(T v) { - if (v instanceof Float) { - return Float.isNaN((Float) v); - } else if (v instanceof Double) { - return Double.isNaN((Double) v); - } else { - return false; - } - } - - @Override - public int compare(final T o1, final T o2) { - final boolean isNull1 = isNull(o1); - final boolean isNull2 = isNull(o2); - - if (isNull1 && isNull2) { - return 0; - } else if (isNull1) { - return -1; - } else if (isNull2) { - return 1; - } - - final boolean isNaN1 = isNanVal(o1); - final boolean isNaN2 = isNanVal(o2); - - // NaN is considered the greatest - if (isNaN1 && isNaN2) { - return 0; - } else if (isNaN1) { - return 1; - } else if (isNaN2) { - return -1; - } - - return o1.compareTo(o2); - } - -} diff --git a/engine/function/src/templates/Sort.ftl b/engine/function/src/templates/Sort.ftl index 4f6ac9f7ed9..00e534a8ddc 100644 --- a/engine/function/src/templates/Sort.ftl +++ b/engine/function/src/templates/Sort.ftl @@ -5,7 +5,6 @@ package io.deephaven.function; import io.deephaven.vector.*; -import io.deephaven.function.comparators.NullNaNAwareComparator; import org.apache.commons.lang3.ArrayUtils; import java.util.Arrays; @@ -51,7 +50,7 @@ public class Sort { * @return sorted values. */ static public > T[] sortObj(final ObjectVector values) { - return sortObj(values, new NullNaNAwareComparator<>()); + return sortObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -83,7 +82,7 @@ public class Sort { */ @SafeVarargs static public > T[] sortObj(final T... values) { - return sortObj(values, new NullNaNAwareComparator<>()); + return sortObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -113,7 +112,7 @@ public class Sort { * @return sorted indices. */ static public > int[] rankObj(final ObjectVector values) { - return rankObj(values, new NullNaNAwareComparator<>()); + return rankObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -141,7 +140,7 @@ public class Sort { */ @SafeVarargs static public > int[] rankObj(final T... values) { - return rankObj(values, new NullNaNAwareComparator<>()); + return rankObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -172,7 +171,7 @@ public class Sort { * @return sorted values. */ static public > T[] sortDescendingObj(final ObjectVector values) { - return sortDescendingObj(values, new NullNaNAwareComparator<>()); + return sortDescendingObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -198,7 +197,7 @@ public class Sort { */ @SafeVarargs static public > T[] sortDescendingObj(final T... values) { - return sortDescendingObj(values, new NullNaNAwareComparator<>()); + return sortDescendingObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -229,7 +228,7 @@ public class Sort { * @return sorted indices. */ static public > int[] rankDescendingObj(final ObjectVector values) { - return rankDescendingObj(values, new NullNaNAwareComparator<>()); + return rankDescendingObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } /** @@ -255,7 +254,7 @@ public class Sort { */ @SafeVarargs static public > int[] rankDescendingObj(final T... values) { - return rankDescendingObj(values, new NullNaNAwareComparator<>()); + return rankDescendingObj(values, Comparator.nullsFirst(Comparator.naturalOrder())); } <#list primitiveTypes as pt> diff --git a/engine/function/src/templates/TestSort.ftl b/engine/function/src/templates/TestSort.ftl index ab3e3dd2066..28c41b106d6 100644 --- a/engine/function/src/templates/TestSort.ftl +++ b/engine/function/src/templates/TestSort.ftl @@ -9,6 +9,7 @@ import io.deephaven.vector.*; import org.apache.commons.lang3.ArrayUtils; import java.math.BigDecimal; +import java.util.Arrays; import static io.deephaven.util.QueryConstants.*; import static io.deephaven.function.Sort.*; @@ -94,6 +95,41 @@ public class TestSort extends BaseArrayTestCase { assertTrue(ArrayUtils.isEmpty(sortedNumbers)); } + public void testObjSortDoubles() { + final Comparable [] a = new Comparable[7]; + a[0] = 7.0; + a[1] = Double.POSITIVE_INFINITY + Double.NEGATIVE_INFINITY; + a[2] = null; + a[3] = Double.POSITIVE_INFINITY; + a[4] = -Math.PI; + a[5] = Double.NEGATIVE_INFINITY; + a[6] = Math.E; + final ObjectVectorDirect values = new ObjectVectorDirect<>(a); + final Comparable[] res = sortObj(values); + assertTrue(Double.isNaN((double)res[6])); + assertEquals(new Comparable[]{null, Double.NEGATIVE_INFINITY, -Math.PI, Math.E, 7.0, Double.POSITIVE_INFINITY}, new ObjectVectorDirect<>(res).subVector(0, 6).toArray()); + + final Comparable[] resd = sortDescendingObj(values); + assertTrue(Double.isNaN((double)resd[0])); + assertEquals(new Comparable[]{Double.POSITIVE_INFINITY, 7.0, Math.E, -Math.PI, Double.NEGATIVE_INFINITY, null}, new ObjectVectorDirect<>(resd).subVector(1, 7).toArray()); + } + + public void testObjRankDoubles() { + final Comparable [] a = new Comparable[7]; + a[0] = 7.0; + a[1] = Double.POSITIVE_INFINITY + Double.NEGATIVE_INFINITY; + a[2] = null; + a[3] = Double.POSITIVE_INFINITY; + a[4] = -Math.PI; + a[5] = Double.NEGATIVE_INFINITY; + a[6] = Math.E; + final ObjectVectorDirect values = new ObjectVectorDirect<>(a); + final int[] res = rankObj(values); + assertEquals(new int[]{2, 5, 4, 6, 0, 3, 1}, res); + + final int[] resd = rankDescendingObj(values); + assertEquals(new int[]{1, 3, 0, 6, 4, 5, 2}, resd); + } <#list primitiveTypes as pt> <#if pt.valueType.isNumber > diff --git a/engine/function/src/test/java/io/deephaven/function/comparators/TestNullNaNAwareComparator.java b/engine/function/src/test/java/io/deephaven/function/comparators/TestNullNaNAwareComparator.java deleted file mode 100644 index 21d93411a67..00000000000 --- a/engine/function/src/test/java/io/deephaven/function/comparators/TestNullNaNAwareComparator.java +++ /dev/null @@ -1,97 +0,0 @@ -// -// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending -// -package io.deephaven.function.comparators; - -import io.deephaven.base.testing.BaseArrayTestCase; - -import java.util.Comparator; - -import static io.deephaven.util.QueryConstants.NULL_DOUBLE; -import static io.deephaven.util.QueryConstants.NULL_FLOAT; - -/** - * Test NullNaNAwareComparator. - */ -public class TestNullNaNAwareComparator extends BaseArrayTestCase { - - public void testDouble() { - final Double v1 = 1.4; - final Double v2 = 2.3; - final Double v3 = NULL_DOUBLE; - final Double v4 = NULL_DOUBLE; - final Double v5 = null; - final Double v6 = Double.NaN; - final Double v7 = Double.NaN; - - final Comparator cmp = new NullNaNAwareComparator<>(); - assertEquals(0, cmp.compare(v1, v1)); - assertEquals(0, cmp.compare(v2, v2)); - assertEquals(0, cmp.compare(v3, v3)); - assertEquals(0, cmp.compare(v4, v4)); - assertEquals(0, cmp.compare(v5, v5)); - assertEquals(0, cmp.compare(v6, v6)); - assertEquals(0, cmp.compare(v7, v7)); - - assertEquals(0, cmp.compare(v3, v4)); - assertEquals(0, cmp.compare(v4, v3)); - assertEquals(0, cmp.compare(v3, v5)); - assertEquals(0, cmp.compare(v5, v3)); - assertEquals(0, cmp.compare(v4, v5)); - assertEquals(0, cmp.compare(v5, v4)); - - assertEquals(0, cmp.compare(v6, v7)); - assertEquals(0, cmp.compare(v7, v6)); - - assertEquals(-1, cmp.compare(v1, v2)); - assertEquals(1, cmp.compare(v2, v1)); - - assertEquals(1, cmp.compare(v1, v3)); - assertEquals(-1, cmp.compare(v3, v1)); - assertEquals(1, cmp.compare(v1, v5)); - assertEquals(-1, cmp.compare(v5, v1)); - - assertEquals(-1, cmp.compare(v1, v6)); - assertEquals(1, cmp.compare(v6, v1)); - } - - public void testFloat() { - final Float v1 = 1.4f; - final Float v2 = 2.3f; - final Float v3 = NULL_FLOAT; - final Float v4 = NULL_FLOAT; - final Float v5 = null; - final Float v6 = Float.NaN; - final Float v7 = Float.NaN; - - final Comparator cmp = new NullNaNAwareComparator<>(); - assertEquals(0, cmp.compare(v1, v1)); - assertEquals(0, cmp.compare(v2, v2)); - assertEquals(0, cmp.compare(v3, v3)); - assertEquals(0, cmp.compare(v4, v4)); - assertEquals(0, cmp.compare(v5, v5)); - assertEquals(0, cmp.compare(v6, v6)); - assertEquals(0, cmp.compare(v7, v7)); - - assertEquals(0, cmp.compare(v3, v4)); - assertEquals(0, cmp.compare(v4, v3)); - assertEquals(0, cmp.compare(v3, v5)); - assertEquals(0, cmp.compare(v5, v3)); - assertEquals(0, cmp.compare(v4, v5)); - assertEquals(0, cmp.compare(v5, v4)); - - assertEquals(0, cmp.compare(v6, v7)); - assertEquals(0, cmp.compare(v7, v6)); - - assertEquals(-1, cmp.compare(v1, v2)); - assertEquals(1, cmp.compare(v2, v1)); - - assertEquals(1, cmp.compare(v1, v3)); - assertEquals(-1, cmp.compare(v3, v1)); - assertEquals(1, cmp.compare(v1, v5)); - assertEquals(-1, cmp.compare(v5, v1)); - - assertEquals(-1, cmp.compare(v1, v6)); - assertEquals(1, cmp.compare(v6, v1)); - } -}