Merge pull request #795 from Kotlin/number-fixes

This fixes Nothing
Kotlin · Jul 31, 2024 · 7ff578a · 7ff578a
2 parents e3ddfcf + cb9125d
commit 7ff578a
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 29 deletions.
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt
@@ -197,15 +197,19 @@ internal fun commonParent(vararg classes: KClass<*>): KClass<*>? = commonParent(
 internal fun Iterable<KClass<*>>.withMostSuperclasses(): KClass<*>? = maxByOrNull { it.allSuperclasses.size }
 
 internal fun Iterable<KClass<*>>.createType(nullable: Boolean, upperBound: KType? = null): KType =
-    if (upperBound == null) {
-        (withMostSuperclasses() ?: Any::class).createStarProjectedType(nullable)
-    } else {
-        val upperClass = upperBound.classifier as KClass<*>
-        val baseClass = filter { it.isSubclassOf(upperClass) }.withMostSuperclasses() ?: withMostSuperclasses()
-        if (baseClass == null) {
-            upperBound.withNullability(nullable)
-        } else {
-            upperBound.projectTo(baseClass).withNullability(nullable)
+    when {
+        !iterator().hasNext() -> upperBound?.withNullability(nullable) ?: nothingType(nullable)
+
+        upperBound == null -> (withMostSuperclasses() ?: Any::class).createStarProjectedType(nullable)
+
+        else -> {
+            val upperClass = upperBound.classifier as KClass<*>
+            val baseClass = filter { it.isSubclassOf(upperClass) }.withMostSuperclasses() ?: withMostSuperclasses()
+            if (baseClass == null) {
+                upperBound.withNullability(nullable)
+            } else {
+                upperBound.projectTo(baseClass).withNullability(nullable)
+            }
         }
     }
 

diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/describe.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/describe.kt
@@ -25,10 +25,10 @@ import org.jetbrains.kotlinx.dataframe.columns.size
 import org.jetbrains.kotlinx.dataframe.columns.values
 import org.jetbrains.kotlinx.dataframe.impl.columns.addPath
 import org.jetbrains.kotlinx.dataframe.impl.columns.asAnyFrameColumn
+import org.jetbrains.kotlinx.dataframe.impl.renderType
 import org.jetbrains.kotlinx.dataframe.index
 import org.jetbrains.kotlinx.dataframe.kind
 import org.jetbrains.kotlinx.dataframe.type
-import kotlin.reflect.jvm.jvmErasure
 
 internal fun describeImpl(cols: List<AnyCol>): DataFrame<ColumnDescription> {
     fun List<AnyCol>.collectAll(atAnyDepth: Boolean): List<AnyCol> =
@@ -65,7 +65,7 @@ internal fun describeImpl(cols: List<AnyCol>): DataFrame<ColumnDescription> {
         if (hasLongPaths) {
             ColumnDescription::path from { it.path() }
         }
-        ColumnDescription::type from { buildTypeName(it) }
+        ColumnDescription::type from { renderType(it.type) }
         ColumnDescription::count from { it.size }
         ColumnDescription::unique from { it.countDistinct() }
         ColumnDescription::nulls from { it.values.count { it == null } }
@@ -94,12 +94,3 @@ internal fun describeImpl(cols: List<AnyCol>): DataFrame<ColumnDescription> {
 
     return df.cast()
 }
-
-private fun buildTypeName(it: AnyCol): String {
-    val rawJavaType = it.type.jvmErasure.simpleName.toString()
-    return if (it.type.isMarkedNullable) {
-        "$rawJavaType?"
-    } else {
-        rawJavaType
-    }
-}
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/math/mean.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/math/mean.kt
@@ -1,6 +1,7 @@
 package org.jetbrains.kotlinx.dataframe.math
 
 import org.jetbrains.kotlinx.dataframe.api.skipNA_default
+import org.jetbrains.kotlinx.dataframe.impl.renderType
 import java.math.BigDecimal
 import kotlin.reflect.KType
 import kotlin.reflect.full.withNullability
@@ -31,7 +32,10 @@ internal fun <T : Number> Sequence<T>.mean(type: KType, skipNA: Boolean = skipNA
 
         Number::class -> (this as Sequence<Number>).map { it.toDouble() }.mean(skipNA)
 
-        else -> throw IllegalArgumentException("Unable to compute mean for type $type")
+        // this means the sequence is empty
+        Nothing::class -> Double.NaN
+
+        else -> throw IllegalArgumentException("Unable to compute the mean for type ${renderType(type)}")
     }
 }
 

diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/math/std.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/math/std.kt
@@ -2,6 +2,7 @@ package org.jetbrains.kotlinx.dataframe.math
 
 import org.jetbrains.kotlinx.dataframe.api.ddof_default
 import org.jetbrains.kotlinx.dataframe.api.skipNA_default
+import org.jetbrains.kotlinx.dataframe.impl.renderType
 import java.math.BigDecimal
 import kotlin.reflect.KType
 import kotlin.reflect.full.withNullability
@@ -13,11 +14,10 @@ internal fun <T : Number> Iterable<T?>.std(
     ddof: Int = ddof_default,
 ): Double {
     if (type.isMarkedNullable) {
-        if (skipNA) {
-            return filterNotNull().std(type.withNullability(false), true, ddof)
-        } else {
-            if (contains(null)) return Double.NaN
-            return std(type.withNullability(false), skipNA, ddof)
+        return when {
+            skipNA -> filterNotNull().std(type = type.withNullability(false), skipNA = true, ddof = ddof)
+            contains(null) -> Double.NaN
+            else -> std(type = type.withNullability(false), skipNA = false, ddof = ddof)
         }
     }
     return when (type.classifier) {
@@ -26,7 +26,8 @@ internal fun <T : Number> Iterable<T?>.std(
         Int::class, Short::class, Byte::class -> (this as Iterable<Int>).std(ddof)
         Long::class -> (this as Iterable<Long>).std(ddof)
         BigDecimal::class -> (this as Iterable<BigDecimal>).std(ddof)
-        else -> throw IllegalArgumentException("Unsupported type ${type.classifier}")
+        Nothing::class -> Double.NaN
+        else -> throw IllegalArgumentException("Unable to compute the std for type ${renderType(type)}")
     }
 }
 

diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/constructors.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/constructors.kt
@@ -1,6 +1,8 @@
 package org.jetbrains.kotlinx.dataframe.api
 
 import io.kotest.matchers.shouldBe
+import org.jetbrains.kotlinx.dataframe.impl.nothingType
+import org.jetbrains.kotlinx.dataframe.type
 import org.junit.Test
 
 class ConstructorsTests {
@@ -24,4 +26,10 @@ class ConstructorsTests {
         df.columnsCount() shouldBe 2
         df.columnNames() shouldBe listOf(column.name(), "${column.name()}1")
     }
+
+    @Test
+    fun `dataFrameOf with nothing columns`() {
+        dataFrameOf("a" to emptyList())["a"].type shouldBe nothingType(false)
+        dataFrameOf("a" to listOf(null))["a"].type shouldBe nothingType(true)
+    }
 }
diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/statistics/BasicMathTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/statistics/BasicMathTests.kt
@@ -1,8 +1,10 @@
 package org.jetbrains.kotlinx.dataframe.statistics
 
 import io.kotest.matchers.shouldBe
+import org.jetbrains.kotlinx.dataframe.DataColumn
 import org.jetbrains.kotlinx.dataframe.api.columnOf
 import org.jetbrains.kotlinx.dataframe.api.mean
+import org.jetbrains.kotlinx.dataframe.impl.nothingType
 import org.junit.Test
 import kotlin.reflect.typeOf
 
@@ -18,5 +20,8 @@ class BasicMathTests {
     fun `mean with nans and nulls`() {
         columnOf(10, 20, Double.NaN, null).mean() shouldBe Double.NaN
         columnOf(10, 20, Double.NaN, null).mean(skipNA = true) shouldBe 15
+
+        DataColumn.createValueColumn("", emptyList<Nothing>(), nothingType(false)).mean() shouldBe Double.NaN
+        DataColumn.createValueColumn("", listOf(null), nothingType(true)).mean() shouldBe Double.NaN
     }
 }
diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/statistics/std.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/statistics/std.kt
@@ -1,11 +1,14 @@
 package org.jetbrains.kotlinx.dataframe.statistics
 
 import io.kotest.matchers.shouldBe
+import org.jetbrains.kotlinx.dataframe.DataColumn
 import org.jetbrains.kotlinx.dataframe.api.columnOf
 import org.jetbrains.kotlinx.dataframe.api.columnTypes
 import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
 import org.jetbrains.kotlinx.dataframe.api.std
+import org.jetbrains.kotlinx.dataframe.impl.nothingType
 import org.jetbrains.kotlinx.dataframe.math.std
+import org.jetbrains.kotlinx.dataframe.type
 import org.junit.Test
 import kotlin.reflect.typeOf
 
@@ -37,4 +40,16 @@ class StdTests {
         df[value].std() shouldBe expected
         df.std { value } shouldBe expected
     }
+
+    @Test
+    fun `std on empty or nullable column`() {
+        val empty = DataColumn.createValueColumn("", emptyList<Nothing>(), nothingType(false))
+        val nullable = DataColumn.createValueColumn("", listOf(null), nothingType(true))
+
+        empty.values().std(empty.type) shouldBe Double.NaN
+        nullable.values().std(nullable.type) shouldBe Double.NaN
+
+        empty.std() shouldBe Double.NaN
+        nullable.std() shouldBe Double.NaN
+    }
 }
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
@@ -7,6 +7,7 @@ import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import org.jetbrains.kotlinx.dataframe.AnyFrame
 import org.jetbrains.kotlinx.dataframe.AnyRow
+import org.jetbrains.kotlinx.dataframe.DataColumn
 import org.jetbrains.kotlinx.dataframe.DataFrame
 import org.jetbrains.kotlinx.dataframe.DataRow
 import org.jetbrains.kotlinx.dataframe.RowExpression
@@ -2196,6 +2197,9 @@ class DataFrameTests : BaseTest() {
     fun `isNumber`() {
         typed.age.isNumber() shouldBe true
         typed.weight.isNumber() shouldBe true
+
+        DataColumn.createValueColumn("a", emptyList<Nothing>(), nothingType(false)).isNumber() shouldBe true
+        DataColumn.createValueColumn("a", listOf(null), nothingType(true)).isNumber() shouldBe true
     }
 
     @Test

diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt
@@ -97,8 +97,8 @@ class UtilTests {
 
     @Test
     fun `createType test`() {
-        emptyList<KClass<*>>().createType(nullable = false) shouldBe typeOf<Any>()
-        emptyList<KClass<*>>().createType(nullable = true) shouldBe typeOf<Any?>()
+        emptyList<KClass<*>>().createType(nullable = false) shouldBe nothingType(nullable = false)
+        emptyList<KClass<*>>().createType(nullable = true) shouldBe nothingType(nullable = true)
 
         listOf(Nothing::class).createType(nullable = false) shouldBe nothingType(nullable = false)
         listOf(Nothing::class).createType(nullable = true) shouldBe nothingType(nullable = true)
@@ -111,6 +111,9 @@ class UtilTests {
 
         listOf(Nothing::class).commonType(false) shouldBe nothingType(nullable = false)
         listOf(Nothing::class).commonType(true) shouldBe nothingType(nullable = true)
+
+        emptyList<KClass<*>>().commonType(false, null) shouldBe nothingType(nullable = false)
+        emptyList<KClass<*>>().commonType(true, null) shouldBe nothingType(nullable = true)
     }
 
     val a = listOf(1, 2.0, "a")
@@ -133,6 +136,7 @@ class UtilTests {
         guessValueType(sequenceOf(1, 2.0, "a", null, listOf(1, 2))) shouldBe typeOf<Any?>()
 
         guessValueType(sequenceOf(null, null)) shouldBe nothingType(nullable = true)
+        guessValueType(emptySequence()) shouldBe nothingType(nullable = false)
 
         guessValueType(sequenceOf(listOf<Int?>(null))) shouldBe typeOf<List<Nothing?>>()
         guessValueType(sequenceOf(emptyList<Int>())) shouldBe typeOf<List<Nothing>>()