Fix valueCounts review cases and preserve empty value column nullability

Author: Kanish2002

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

@@ -149,10 +149,9 @@ public fun <T> DataColumn<T>.valueCounts(
     if (dropNA) grouped = grouped.filter { !it.first.isNA }
     val nulls = if (dropNA) false else hasNulls()
     val values =
-        if (grouped.isEmpty()) {
-            // #2034, keep the original name and type of the column even when the input is empty
-            // nullability is irrelevant, but let's keep it for consistency
-            take(0).setNullable(nulls)
+        if (grouped.isEmpty() && !isValueColumn()) {
+            // Preserve nested columns when there are no counted values.
+            take(0)
         } else {
             DataColumn.createByType(name(), grouped.map { it.first }, type().withNullability(nulls))
         }

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt

@@ -1,6 +1,7 @@
 package org.jetbrains.kotlinx.dataframe.api
 
 import io.kotest.matchers.shouldBe
+import org.jetbrains.kotlinx.dataframe.DataColumn
 import org.jetbrains.kotlinx.dataframe.nrow
 import org.junit.Test
 import kotlin.reflect.typeOf
@@ -104,6 +105,30 @@ class ValueCountsTests {
         result.rowsCount() shouldBe 0
     }
 
+    @Test
+    fun `value counts of an empty nullable value column preserves type when NA values are included`() {
+        val input = DataColumn.createByType("city", emptyList<String?>(), typeOf<String?>())
+        val result = input.valueCounts(dropNA = false)
+
+        result.columnNames() shouldBe listOf("city", "count")
+        result.nrow shouldBe 0
+        result["city"].isValueColumn() shouldBe true
+        result["city"].type() shouldBe typeOf<String?>()
+        result["count"].type() shouldBe typeOf<Int>()
+    }
+
+    @Test
+    fun `value counts of an empty nullable value column drops nullability when NA values are excluded`() {
+        val input = DataColumn.createByType("city", emptyList<String?>(), typeOf<String?>())
+        val result = input.valueCounts()
+
+        result.columnNames() shouldBe listOf("city", "count")
+        result.nrow shouldBe 0
+        result["city"].isValueColumn() shouldBe true
+        result["city"].type() shouldBe typeOf<String>()
+        result["count"].type() shouldBe typeOf<Int>()
+    }
+
     @Test
     fun `value counts of a column with a result column name clashing with the column name`() {
         val count by columnOf("a", "b", "a")
@@ -288,12 +313,12 @@ class ValueCountsTests {
 
     @Test
     fun `value counts retains selected columns when every row is dropped`() {
-        val input = dataFrameOf("city" to columnOf<String?>(null, null))
+        val input = dataFrameOf("city" to columnOf("London", null)).drop(1)
         val result = input.valueCounts("city")
 
         result.columnNames() shouldBe listOf("city", "count")
         result.nrow shouldBe 0
-        result["city"].type() shouldBe typeOf<String?>() // TODO #2034
+        result["city"].type() shouldBe typeOf<String>()
         result["count"].type() shouldBe typeOf<Int>()
     }
 
@@ -305,8 +330,8 @@ class ValueCountsTests {
         result.columnNames() shouldBe listOf("details", "count")
         result["details"].asColumnGroup().columnNames() shouldBe listOf("name", "city")
         result.nrow shouldBe 0
-        result["details"].asColumnGroup()["name"].type() shouldBe typeOf<String?>() // TODO #2034
-        result["details"].asColumnGroup()["city"].type() shouldBe typeOf<String?>() // TODO #2034
+        result["details"].asColumnGroup()["name"].type() shouldBe typeOf<String>()
+        result["details"].asColumnGroup()["city"].type() shouldBe typeOf<String>()
         result["count"].type() shouldBe typeOf<Int>()
     }
 

docs/StardustDocs/topics/valueCounts.md

@@ -78,9 +78,6 @@ df.valueCounts("name", dropNA = false)
 
 <!---END-->
 
-If no observations are counted, the result has zero rows and retains the observed-value columns and count column.
-This includes empty input and rows excluded by `dropNA`.
-
 This operation can also be applied to a [`DataColumn`](DataColumn.md):
 
 <!---FUN valueCountsColumn-->