Skip to content

Commit a38edf4

Browse files
JolanrensenJolanrensen
authored
Merge pull request #726 from Kotlin/is-comparable-fix
`isComparable()` fix for double `describe()`
2 parents 7a219dc + 21c5c2b commit a38edf4

File tree

6 files changed

+150
-8
lines changed

6 files changed

+150
-8
lines changed

‎core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package org.jetbrains.kotlinx.dataframe.api
22

33
import org.jetbrains.kotlinx.dataframe.AnyCol
44
import org.jetbrains.kotlinx.dataframe.columns.ColumnKind
5+
import org.jetbrains.kotlinx.dataframe.impl.isNothing
6+
import org.jetbrains.kotlinx.dataframe.impl.projectTo
57
import org.jetbrains.kotlinx.dataframe.type
68
import org.jetbrains.kotlinx.dataframe.typeClass
79
import kotlin.reflect.KClass
810
import kotlin.reflect.KType
11+
import kotlin.reflect.KTypeProjection
912
import kotlin.reflect.full.isSubclassOf
1013
import kotlin.reflect.full.isSubtypeOf
1114
import kotlin.reflect.typeOf
@@ -16,7 +19,8 @@ public fun AnyCol.isFrameColumn(): Boolean = kind() == ColumnKind.Frame
1619

1720
public fun AnyCol.isValueColumn(): Boolean = kind() == ColumnKind.Value
1821

19-
public fun AnyCol.isSubtypeOf(type: KType): Boolean = this.type.isSubtypeOf(type) && (!this.type.isMarkedNullable || type.isMarkedNullable)
22+
public fun AnyCol.isSubtypeOf(type: KType): Boolean =
23+
this.type.isSubtypeOf(type) && (!this.type.isMarkedNullable || type.isMarkedNullable)
2024

2125
public inline fun <reified T> AnyCol.isSubtypeOf(): Boolean = isSubtypeOf(typeOf<T>())
2226

@@ -26,9 +30,23 @@ public fun AnyCol.isNumber(): Boolean = isSubtypeOf<Number?>()
2630

2731
public fun AnyCol.isList(): Boolean = typeClass == List::class
2832

29-
public fun AnyCol.isComparable(): Boolean = isSubtypeOf<Comparable<*>?>()
33+
/**
34+
* Returns `true` if [this] column is comparable, i.e. its type is a subtype of [Comparable] and its
35+
* type argument is not [Nothing].
36+
*/
37+
public fun AnyCol.isComparable(): Boolean =
38+
isSubtypeOf<Comparable<*>?>() &&
39+
type().projectTo(Comparable::class).arguments[0].let {
40+
it != KTypeProjection.STAR &&
41+
it.type?.isNothing != true
42+
}
3043

3144
@PublishedApi
3245
internal fun AnyCol.isPrimitive(): Boolean = typeClass.isPrimitive()
3346

34-
internal fun KClass<*>.isPrimitive(): Boolean = isSubclassOf(Number::class) || this == String::class || this == Char::class || this == Array::class || isSubclassOf(Collection::class)
47+
internal fun KClass<*>.isPrimitive(): Boolean =
48+
isSubclassOf(Number::class) ||
49+
this == String::class ||
50+
this == Char::class ||
51+
this == Array::class ||
52+
isSubclassOf(Collection::class)

‎core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import kotlin.reflect.KType
1212
import kotlin.reflect.KTypeParameter
1313
import kotlin.reflect.KTypeProjection
1414
import kotlin.reflect.KVariance
15-
import kotlin.reflect.KVariance.*
15+
import kotlin.reflect.KVariance.IN
16+
import kotlin.reflect.KVariance.INVARIANT
17+
import kotlin.reflect.KVariance.OUT
1618
import kotlin.reflect.KVisibility
1719
import kotlin.reflect.full.allSuperclasses
1820
import kotlin.reflect.full.createType
@@ -463,6 +465,9 @@ internal fun guessValueType(values: Sequence<Any?>, upperBound: KType? = null, l
463465
}
464466
}
465467

468+
internal val KType.isNothing: Boolean
469+
get() = classifier == Nothing::class
470+
466471
internal fun nothingType(nullable: Boolean): KType =
467472
if (nullable) {
468473
typeOf<List<Nothing?>>()

‎core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import io.kotest.matchers.shouldBe
77
import io.kotest.matchers.shouldNotBe
88
import org.jetbrains.kotlinx.dataframe.AnyFrame
99
import org.jetbrains.kotlinx.dataframe.AnyRow
10+
import org.jetbrains.kotlinx.dataframe.DataColumn
1011
import org.jetbrains.kotlinx.dataframe.DataFrame
1112
import org.jetbrains.kotlinx.dataframe.DataRow
1213
import org.jetbrains.kotlinx.dataframe.RowExpression
@@ -79,6 +80,7 @@ import org.jetbrains.kotlinx.dataframe.api.intoColumns
7980
import org.jetbrains.kotlinx.dataframe.api.intoList
8081
import org.jetbrains.kotlinx.dataframe.api.intoRows
8182
import org.jetbrains.kotlinx.dataframe.api.isColumnGroup
83+
import org.jetbrains.kotlinx.dataframe.api.isComparable
8284
import org.jetbrains.kotlinx.dataframe.api.isEmpty
8385
import org.jetbrains.kotlinx.dataframe.api.isFrameColumn
8486
import org.jetbrains.kotlinx.dataframe.api.isNA
@@ -119,6 +121,7 @@ import org.jetbrains.kotlinx.dataframe.api.rename
119121
import org.jetbrains.kotlinx.dataframe.api.reorderColumnsByName
120122
import org.jetbrains.kotlinx.dataframe.api.replace
121123
import org.jetbrains.kotlinx.dataframe.api.rows
124+
import org.jetbrains.kotlinx.dataframe.api.schema
122125
import org.jetbrains.kotlinx.dataframe.api.select
123126
import org.jetbrains.kotlinx.dataframe.api.single
124127
import org.jetbrains.kotlinx.dataframe.api.sortBy
@@ -165,18 +168,22 @@ import org.jetbrains.kotlinx.dataframe.exceptions.ExcessiveColumnsException
165168
import org.jetbrains.kotlinx.dataframe.exceptions.TypeConversionException
166169
import org.jetbrains.kotlinx.dataframe.get
167170
import org.jetbrains.kotlinx.dataframe.hasNulls
171+
import org.jetbrains.kotlinx.dataframe.impl.DataFrameImpl
168172
import org.jetbrains.kotlinx.dataframe.impl.DataFrameSize
169173
import org.jetbrains.kotlinx.dataframe.impl.api.convertToImpl
170174
import org.jetbrains.kotlinx.dataframe.impl.between
171175
import org.jetbrains.kotlinx.dataframe.impl.columns.isMissingColumn
172176
import org.jetbrains.kotlinx.dataframe.impl.emptyPath
173177
import org.jetbrains.kotlinx.dataframe.impl.getColumnsImpl
178+
import org.jetbrains.kotlinx.dataframe.impl.isNothing
174179
import org.jetbrains.kotlinx.dataframe.impl.nothingType
180+
import org.jetbrains.kotlinx.dataframe.impl.projectTo
175181
import org.jetbrains.kotlinx.dataframe.impl.trackColumnAccess
176182
import org.jetbrains.kotlinx.dataframe.index
177183
import org.jetbrains.kotlinx.dataframe.io.renderValueForStdout
178184
import org.jetbrains.kotlinx.dataframe.kind
179185
import org.jetbrains.kotlinx.dataframe.math.mean
186+
import org.jetbrains.kotlinx.dataframe.name
180187
import org.jetbrains.kotlinx.dataframe.ncol
181188
import org.jetbrains.kotlinx.dataframe.nrow
182189
import org.jetbrains.kotlinx.dataframe.size
@@ -2358,6 +2365,47 @@ class DataFrameTests : BaseTest() {
23582365
desc.print()
23592366
}
23602367

2368+
@DataSchema
2369+
data class ComparableTest(
2370+
val int: Int,
2371+
val comparableInt: Comparable<Int>,
2372+
val string: String,
2373+
val comparableString: Comparable<String>,
2374+
val comparableStar: Comparable<*>,
2375+
val comparableNothing: Comparable<Nothing>,
2376+
)
2377+
2378+
@Test
2379+
fun `is comparable`() {
2380+
val df = listOf(
2381+
ComparableTest(1, 1, "a", "a", 1, 1),
2382+
ComparableTest(2, 2, "b", "b", "2", "2"),
2383+
).toDataFrame()
2384+
2385+
df.int.isComparable() shouldBe true
2386+
df.comparableInt.isComparable() shouldBe true
2387+
df.string.isComparable() shouldBe true
2388+
df.comparableString.isComparable() shouldBe true
2389+
df.comparableStar.isComparable() shouldBe false
2390+
df.comparableNothing.isComparable() shouldBe false
2391+
}
2392+
2393+
@Test
2394+
fun `describe twice minimal`() {
2395+
val df = dataFrameOf("a", "b")(1, "foo", 3, "bar")
2396+
val desc1 = df.describe()
2397+
val desc2 = desc1.describe()
2398+
desc2::class shouldBe DataFrameImpl::class
2399+
}
2400+
2401+
@Test
2402+
fun `describe twice`() {
2403+
val df = typed.group { age and weight }.into("info").groupBy { city }.toDataFrame()
2404+
val desc1 = df.describe()
2405+
val desc2 = desc1.describe()
2406+
desc2::class shouldBe DataFrameImpl::class
2407+
}
2408+
23612409
@Test
23622410
fun `index by column accessor`() {
23632411
val col = listOf(1, 2, 3, 4, 5).toColumn("name")

‎core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/DataColumnType.kt

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@ package org.jetbrains.kotlinx.dataframe.api
22

33
import org.jetbrains.kotlinx.dataframe.AnyCol
44
import org.jetbrains.kotlinx.dataframe.columns.ColumnKind
5+
import org.jetbrains.kotlinx.dataframe.impl.isNothing
6+
import org.jetbrains.kotlinx.dataframe.impl.projectTo
57
import org.jetbrains.kotlinx.dataframe.type
68
import org.jetbrains.kotlinx.dataframe.typeClass
79
import kotlin.reflect.KClass
810
import kotlin.reflect.KType
11+
import kotlin.reflect.KTypeProjection
912
import kotlin.reflect.full.isSubclassOf
1013
import kotlin.reflect.full.isSubtypeOf
1114
import kotlin.reflect.typeOf
@@ -16,7 +19,8 @@ public fun AnyCol.isFrameColumn(): Boolean = kind() == ColumnKind.Frame
1619

1720
public fun AnyCol.isValueColumn(): Boolean = kind() == ColumnKind.Value
1821

19-
public fun AnyCol.isSubtypeOf(type: KType): Boolean = this.type.isSubtypeOf(type) && (!this.type.isMarkedNullable || type.isMarkedNullable)
22+
public fun AnyCol.isSubtypeOf(type: KType): Boolean =
23+
this.type.isSubtypeOf(type) && (!this.type.isMarkedNullable || type.isMarkedNullable)
2024

2125
public inline fun <reified T> AnyCol.isSubtypeOf(): Boolean = isSubtypeOf(typeOf<T>())
2226

@@ -26,9 +30,23 @@ public fun AnyCol.isNumber(): Boolean = isSubtypeOf<Number?>()
2630

2731
public fun AnyCol.isList(): Boolean = typeClass == List::class
2832

29-
public fun AnyCol.isComparable(): Boolean = isSubtypeOf<Comparable<*>?>()
33+
/**
34+
* Returns `true` if [this] column is comparable, i.e. its type is a subtype of [Comparable] and its
35+
* type argument is not [Nothing].
36+
*/
37+
public fun AnyCol.isComparable(): Boolean =
38+
isSubtypeOf<Comparable<*>?>() &&
39+
type().projectTo(Comparable::class).arguments[0].let {
40+
it != KTypeProjection.STAR &&
41+
it.type?.isNothing != true
42+
}
3043

3144
@PublishedApi
3245
internal fun AnyCol.isPrimitive(): Boolean = typeClass.isPrimitive()
3346

34-
internal fun KClass<*>.isPrimitive(): Boolean = isSubclassOf(Number::class) || this == String::class || this == Char::class || this == Array::class || isSubclassOf(Collection::class)
47+
internal fun KClass<*>.isPrimitive(): Boolean =
48+
isSubclassOf(Number::class) ||
49+
this == String::class ||
50+
this == Char::class ||
51+
this == Array::class ||
52+
isSubclassOf(Collection::class)

‎core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import kotlin.reflect.KType
1212
import kotlin.reflect.KTypeParameter
1313
import kotlin.reflect.KTypeProjection
1414
import kotlin.reflect.KVariance
15-
import kotlin.reflect.KVariance.*
15+
import kotlin.reflect.KVariance.IN
16+
import kotlin.reflect.KVariance.INVARIANT
17+
import kotlin.reflect.KVariance.OUT
1618
import kotlin.reflect.KVisibility
1719
import kotlin.reflect.full.allSuperclasses
1820
import kotlin.reflect.full.createType
@@ -463,6 +465,9 @@ internal fun guessValueType(values: Sequence<Any?>, upperBound: KType? = null, l
463465
}
464466
}
465467

468+
internal val KType.isNothing: Boolean
469+
get() = classifier == Nothing::class
470+
466471
internal fun nothingType(nullable: Boolean): KType =
467472
if (nullable) {
468473
typeOf<List<Nothing?>>()

‎core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import io.kotest.matchers.shouldBe
77
import io.kotest.matchers.shouldNotBe
88
import org.jetbrains.kotlinx.dataframe.AnyFrame
99
import org.jetbrains.kotlinx.dataframe.AnyRow
10+
import org.jetbrains.kotlinx.dataframe.DataColumn
1011
import org.jetbrains.kotlinx.dataframe.DataFrame
1112
import org.jetbrains.kotlinx.dataframe.DataRow
1213
import org.jetbrains.kotlinx.dataframe.RowExpression
@@ -79,6 +80,7 @@ import org.jetbrains.kotlinx.dataframe.api.intoColumns
7980
import org.jetbrains.kotlinx.dataframe.api.intoList
8081
import org.jetbrains.kotlinx.dataframe.api.intoRows
8182
import org.jetbrains.kotlinx.dataframe.api.isColumnGroup
83+
import org.jetbrains.kotlinx.dataframe.api.isComparable
8284
import org.jetbrains.kotlinx.dataframe.api.isEmpty
8385
import org.jetbrains.kotlinx.dataframe.api.isFrameColumn
8486
import org.jetbrains.kotlinx.dataframe.api.isNA
@@ -119,6 +121,7 @@ import org.jetbrains.kotlinx.dataframe.api.rename
119121
import org.jetbrains.kotlinx.dataframe.api.reorderColumnsByName
120122
import org.jetbrains.kotlinx.dataframe.api.replace
121123
import org.jetbrains.kotlinx.dataframe.api.rows
124+
import org.jetbrains.kotlinx.dataframe.api.schema
122125
import org.jetbrains.kotlinx.dataframe.api.select
123126
import org.jetbrains.kotlinx.dataframe.api.single
124127
import org.jetbrains.kotlinx.dataframe.api.sortBy
@@ -165,18 +168,22 @@ import org.jetbrains.kotlinx.dataframe.exceptions.ExcessiveColumnsException
165168
import org.jetbrains.kotlinx.dataframe.exceptions.TypeConversionException
166169
import org.jetbrains.kotlinx.dataframe.get
167170
import org.jetbrains.kotlinx.dataframe.hasNulls
171+
import org.jetbrains.kotlinx.dataframe.impl.DataFrameImpl
168172
import org.jetbrains.kotlinx.dataframe.impl.DataFrameSize
169173
import org.jetbrains.kotlinx.dataframe.impl.api.convertToImpl
170174
import org.jetbrains.kotlinx.dataframe.impl.between
171175
import org.jetbrains.kotlinx.dataframe.impl.columns.isMissingColumn
172176
import org.jetbrains.kotlinx.dataframe.impl.emptyPath
173177
import org.jetbrains.kotlinx.dataframe.impl.getColumnsImpl
178+
import org.jetbrains.kotlinx.dataframe.impl.isNothing
174179
import org.jetbrains.kotlinx.dataframe.impl.nothingType
180+
import org.jetbrains.kotlinx.dataframe.impl.projectTo
175181
import org.jetbrains.kotlinx.dataframe.impl.trackColumnAccess
176182
import org.jetbrains.kotlinx.dataframe.index
177183
import org.jetbrains.kotlinx.dataframe.io.renderValueForStdout
178184
import org.jetbrains.kotlinx.dataframe.kind
179185
import org.jetbrains.kotlinx.dataframe.math.mean
186+
import org.jetbrains.kotlinx.dataframe.name
180187
import org.jetbrains.kotlinx.dataframe.ncol
181188
import org.jetbrains.kotlinx.dataframe.nrow
182189
import org.jetbrains.kotlinx.dataframe.size
@@ -2358,6 +2365,47 @@ class DataFrameTests : BaseTest() {
23582365
desc.print()
23592366
}
23602367

2368+
@DataSchema
2369+
data class ComparableTest(
2370+
val int: Int,
2371+
val comparableInt: Comparable<Int>,
2372+
val string: String,
2373+
val comparableString: Comparable<String>,
2374+
val comparableStar: Comparable<*>,
2375+
val comparableNothing: Comparable<Nothing>,
2376+
)
2377+
2378+
@Test
2379+
fun `is comparable`() {
2380+
val df = listOf(
2381+
ComparableTest(1, 1, "a", "a", 1, 1),
2382+
ComparableTest(2, 2, "b", "b", "2", "2"),
2383+
).toDataFrame()
2384+
2385+
df.int.isComparable() shouldBe true
2386+
df.comparableInt.isComparable() shouldBe true
2387+
df.string.isComparable() shouldBe true
2388+
df.comparableString.isComparable() shouldBe true
2389+
df.comparableStar.isComparable() shouldBe false
2390+
df.comparableNothing.isComparable() shouldBe false
2391+
}
2392+
2393+
@Test
2394+
fun `describe twice minimal`() {
2395+
val df = dataFrameOf("a", "b")(1, "foo", 3, "bar")
2396+
val desc1 = df.describe()
2397+
val desc2 = desc1.describe()
2398+
desc2::class shouldBe DataFrameImpl::class
2399+
}
2400+
2401+
@Test
2402+
fun `describe twice`() {
2403+
val df = typed.group { age and weight }.into("info").groupBy { city }.toDataFrame()
2404+
val desc1 = df.describe()
2405+
val desc2 = desc1.describe()
2406+
desc2::class shouldBe DataFrameImpl::class
2407+
}
2408+
23612409
@Test
23622410
fun `index by column accessor`() {
23632411
val col = listOf(1, 2, 3, 4, 5).toColumn("name")

0 commit comments

Comments
 (0)