Skip to content

Fixed valuesAreComparable() and tests #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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]`<in T>(?)`
*/
public fun AnyCol.valuesAreComparable(): Boolean =
isSubtypeOf<Comparable<*>?>() &&
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2462,13 +2462,32 @@ class DataFrameTests : BaseTest() {
).toDataFrame()

df.int.valuesAreComparable() shouldBe true
df.comparableInt.valuesAreComparable() shouldBe true
// Comparable<Int> is not comparable to Comparable<Int>
df.comparableInt.valuesAreComparable() shouldBe false
df.string.valuesAreComparable() shouldBe true
df.comparableString.valuesAreComparable() shouldBe true
// Comparable<String> is not comparable to Comparable<String>
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<Int> {
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<Comparable<Int>>
// 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")
Expand Down