Skip to content

Commit 69a6f4b

Browse files
committed
expanded tests, rewrote fix a tiny bit and removed unnecessary kdocs. The new behavior is expected, no need to mention it
1 parent bef7487 commit 69a6f4b

2 files changed

Lines changed: 15 additions & 8 deletions

File tree

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ private typealias ValueCountsParams = Nothing
4949
* - the distinct combinations of these values,
5050
* - a new [Int] column with the number of occurrences of each combination.
5151
* This column is called `"count"` by default unless it is overridden by [resultColumn\].
52-
* If no rows are counted, the result still contains the selected top-level columns and the count column.
5352
*
5453
* By default, the result is sorted by count in descending order (the most frequent combination first),
5554
* and a row is not counted at all if any of its selected values is {@include [NALink]}.
@@ -100,7 +99,6 @@ internal val defaultCountColumnName: String = ValueCount::count.name
10099
* - the column with the distinct values of the original [DataColumn]
101100
* - a new [Int] column with the number of occurrences of each value.
102101
* This column is called `"count"` by default unless it is overridden by [resultColumn].
103-
* If no values are counted, the result still contains the observed-value column and the count column.
104102
*
105103
* By default, the result is sorted by count in descending order (the most frequent value first),
106104
* and {@include [NALink]} values (`null`s and `NaN`s) are not counted.
@@ -150,11 +148,14 @@ public fun <T> DataColumn<T>.valueCounts(
150148
}
151149
if (dropNA) grouped = grouped.filter { !it.first.isNA }
152150
val nulls = if (dropNA) false else hasNulls()
153-
val values = if (grouped.isEmpty()) {
154-
get(emptyList<Int>())
155-
} else {
156-
DataColumn.createByType(name(), grouped.map { it.first }, type().withNullability(nulls))
157-
}
151+
val values =
152+
if (grouped.isEmpty()) {
153+
// #2034, keep the original name and type of the column even when the input is empty
154+
// nullability is irrelevant, but let's keep it for consistency
155+
take(0).setNullable(nulls)
156+
} else {
157+
DataColumn.createByType(name(), grouped.map { it.first }, type().withNullability(nulls))
158+
}
158159
val countName = if (resultColumn == name()) resultColumn + "1" else resultColumn
159160
val counts = DataColumn.createByType(countName, grouped.map { it.second }, typeOf<Int>())
160161
return dataFrameOf(values, counts).cast()

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ class ValueCountsTests {
266266
}
267267

268268
@Test
269-
fun `value counts of an empty DataFrame retains all observed columns`() {
269+
fun `value counts of an empty DataFrame retains all columns`() {
270270
val result = df.take(0).valueCounts()
271271

272272
result.columnNames() shouldBe listOf("name", "city", "age", "count")
@@ -283,6 +283,7 @@ class ValueCountsTests {
283283
result.columnNames() shouldBe listOf("city", "count")
284284
result.nrow shouldBe 0
285285
result["city"].type() shouldBe typeOf<String>()
286+
result["count"].type() shouldBe typeOf<Int>()
286287
}
287288

288289
@Test
@@ -292,6 +293,8 @@ class ValueCountsTests {
292293

293294
result.columnNames() shouldBe listOf("city", "count")
294295
result.nrow shouldBe 0
296+
result["city"].type() shouldBe typeOf<String?>() // TODO #2034
297+
result["count"].type() shouldBe typeOf<Int>()
295298
}
296299

297300
@Test
@@ -302,6 +305,9 @@ class ValueCountsTests {
302305
result.columnNames() shouldBe listOf("details", "count")
303306
result["details"].asColumnGroup().columnNames() shouldBe listOf("name", "city")
304307
result.nrow shouldBe 0
308+
result["details"].asColumnGroup()["name"].type() shouldBe typeOf<String?>() // TODO #2034
309+
result["details"].asColumnGroup()["city"].type() shouldBe typeOf<String?>() // TODO #2034
310+
result["count"].type() shouldBe typeOf<Int>()
305311
}
306312

307313
// endregion

0 commit comments

Comments
 (0)