diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt index 4bd0ad626c..8547f2a060 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt @@ -7,8 +7,6 @@ import org.jetbrains.kotlinx.dataframe.columns.ColumnGroup import org.jetbrains.kotlinx.dataframe.columns.ColumnKind import org.jetbrains.kotlinx.dataframe.columns.FrameColumn import org.jetbrains.kotlinx.dataframe.columns.ValueColumn -import org.jetbrains.kotlinx.dataframe.impl.isNothing -import org.jetbrains.kotlinx.dataframe.impl.projectTo import org.jetbrains.kotlinx.dataframe.type import org.jetbrains.kotlinx.dataframe.typeClass import org.jetbrains.kotlinx.dataframe.util.IS_COMPARABLE @@ -21,8 +19,11 @@ import kotlin.contracts.contract import kotlin.reflect.KClass import kotlin.reflect.KType import kotlin.reflect.KTypeProjection +import kotlin.reflect.KVariance +import kotlin.reflect.full.createType import kotlin.reflect.full.isSubclassOf import kotlin.reflect.full.isSubtypeOf +import kotlin.reflect.full.withNullability import kotlin.reflect.typeOf public fun AnyCol.isColumnGroup(): Boolean { @@ -62,21 +63,24 @@ public fun AnyCol.isList(): Boolean = typeClass == List::class public fun AnyCol.isComparable(): Boolean = valuesAreComparable() /** - * Returns `true` if [this] column is inter-comparable, i.e. + * Returns `true` if [this] column is intra-comparable, i.e. * its values can be compared with each other and thus ordered. * * If true, operations like [`min()`][AnyCol.min], [`max()`][AnyCol.max], [`median()`][AnyCol.median], etc. * will work. * - * Technically, this means the values' common type is a subtype of [Comparable] with - * the type argument not being [Nothing]. + * Technically, this means the values' common type `T(?)` is a subtype of [Comparable]`(?)` */ public fun AnyCol.valuesAreComparable(): Boolean = - isSubtypeOf?>() && - type().projectTo(Comparable::class).arguments[0].let { - it != KTypeProjection.STAR && - it.type?.isNothing != true - } + isValueColumn() && + isSubtypeOf( + Comparable::class.createType( + arguments = listOf( + KTypeProjection(KVariance.IN, type().withNullability(false)), + ), + nullable = hasNulls(), + ), + ) @PublishedApi internal fun AnyCol.isPrimitive(): Boolean = typeClass.isPrimitive() diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt index 5bc715e078..7f5886bbe5 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt @@ -2462,13 +2462,32 @@ class DataFrameTests : BaseTest() { ).toDataFrame() df.int.valuesAreComparable() shouldBe true - df.comparableInt.valuesAreComparable() shouldBe true + // Comparable is not comparable to Comparable + df.comparableInt.valuesAreComparable() shouldBe false df.string.valuesAreComparable() shouldBe true - df.comparableString.valuesAreComparable() shouldBe true + // Comparable is not comparable to Comparable + df.comparableString.valuesAreComparable() shouldBe false df.comparableStar.valuesAreComparable() shouldBe false df.comparableNothing.valuesAreComparable() shouldBe false } + // https://github.com/Kotlin/dataframe/pull/1077#discussion_r1981352374 + @Test + fun `values are comparable difficult`() { + val i = 1 + val i1 = object : Comparable { + override fun compareTo(other: Int): Int = other + + override fun toString(): String = "i1" + } + val col by columnOf(i, i1) + + // We cannot calculate min/max for this column because Int does not implement Comparable> + // aka i1.compareTo(i) would work but i.compareTo(i1) would not + dataFrameOf(col).max().isEmpty() shouldBe true + dataFrameOf(col).min().isEmpty() shouldBe true + } + @Test fun `describe twice minimal`() { val df = dataFrameOf("a", "b")(1, "foo", 3, "bar")