Skip to content

Commit 4f8c867

Browse files
JolanrensenJolanrensen
authored
Merge pull request #1551 from Kotlin/nothing-cols
Nothing cols fixes
2 parents fc5033c + c75a72d commit 4f8c867

File tree

12 files changed

+149
-40
lines changed

12 files changed

+149
-40
lines changed

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import org.jetbrains.kotlinx.dataframe.impl.columns.addPath
2424
import org.jetbrains.kotlinx.dataframe.impl.columns.createColumnGuessingType
2525
import org.jetbrains.kotlinx.dataframe.impl.columns.toColumnKind
2626
import org.jetbrains.kotlinx.dataframe.impl.getValuesType
27+
import org.jetbrains.kotlinx.dataframe.impl.nothingType
2728
import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema
2829
import org.jetbrains.kotlinx.dataframe.util.CHUNKED_IMPL_IMPORT
2930
import org.jetbrains.kotlinx.dataframe.util.CREATE
@@ -216,8 +217,18 @@ public interface DataColumn<out T> : BaseColumn<T> {
216217
infer: Infer = Infer.None,
217218
): DataColumn<T> = createByType(name, values, typeOf<T>(), infer)
218219

219-
/** Creates an empty [DataColumn] with given [name]. */
220-
public fun empty(name: String = ""): AnyCol = createValueColumn(name, emptyList<Unit>(), typeOf<Unit>())
220+
/**
221+
* Creates an empty [DataColumn] with given [name] of type [Nothing].
222+
* If you want to specify another type, use [`emptyOf<T>()`][emptyOf].
223+
*
224+
* @see emptyOf
225+
*/
226+
public fun empty(name: String = ""): DataColumn<Nothing> =
227+
createValueColumn(name, emptyList<Unit>(), nothingType).cast()
228+
229+
/** Creates an empty [DataColumn] of type [T] with given [name]. */
230+
public inline fun <reified T> emptyOf(name: String = ""): DataColumn<T> =
231+
createValueColumn(name, emptyList<T>(), typeOf<T>()).cast()
221232

222233
// region deprecated
223234

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import org.jetbrains.kotlinx.dataframe.impl.columns.TransformableColumnSet
3030
import org.jetbrains.kotlinx.dataframe.impl.columns.addPath
3131
import org.jetbrains.kotlinx.dataframe.impl.columns.onResolve
3232
import org.jetbrains.kotlinx.dataframe.impl.columns.transform
33+
import org.jetbrains.kotlinx.dataframe.impl.nullableNothingType
3334
import org.jetbrains.kotlinx.dataframe.impl.owner
3435
import org.jetbrains.kotlinx.dataframe.util.DEPRECATED_ACCESS_API
3536
import kotlin.reflect.KProperty
@@ -40,7 +41,10 @@ import kotlin.reflect.KProperty
4041
public fun <T> DataColumn<T>.all(predicate: Predicate<T>): Boolean = values.all(predicate)
4142

4243
/** Returns `true` if all [values] are `null` or [values] is empty. */
43-
public fun <C> DataColumn<C>.allNulls(): Boolean = size == 0 || all { it == null }
44+
public fun <C> DataColumn<C>.allNulls(): Boolean =
45+
size == 0 ||
46+
type() == nullableNothingType ||
47+
all { it == null }
4448

4549
// endregion
4650

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ public interface ColsOfColumnsSelectionDsl {
9696
*
9797
* This function operates solely on columns at the top-level.
9898
*
99+
* __NOTE:__ Null-filled columns of type [Nothing?][Nothing] will be included when selecting [`colsOf`][colsOf]`<T?>()`.
100+
* This is because [Nothing][Nothing] is considered a subtype of all other types in Kotlin.
101+
* To exclude these columns, call `.`[filter][ColumnsSelectionDsl.filter]` { !it.`[allNulls][DataColumn.allNulls]`() }`
102+
* after it.
103+
*
99104
* ### Check out: [Grammar]
100105
*
101106
* #### For example:

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

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,18 @@ package org.jetbrains.kotlinx.dataframe.impl
33
import org.jetbrains.kotlinx.dataframe.AnyFrame
44
import org.jetbrains.kotlinx.dataframe.AnyRow
55
import org.jetbrains.kotlinx.dataframe.DataColumn
6-
import org.jetbrains.kotlinx.dataframe.DataFrame
7-
import org.jetbrains.kotlinx.dataframe.DataRow
6+
import org.jetbrains.kotlinx.dataframe.api.asDataColumn
7+
import org.jetbrains.kotlinx.dataframe.api.cast
88
import org.jetbrains.kotlinx.dataframe.api.concat
99
import org.jetbrains.kotlinx.dataframe.api.toDataFrame
1010
import org.jetbrains.kotlinx.dataframe.impl.columns.createColumnGuessingType
1111
import kotlin.reflect.KClass
1212
import kotlin.reflect.KType
1313
import kotlin.reflect.full.isSubclassOf
14+
import kotlin.reflect.full.isSubtypeOf
1415
import kotlin.reflect.full.withNullability
1516
import kotlin.reflect.jvm.jvmErasure
17+
import kotlin.reflect.typeOf
1618

1719
public interface DataCollector<T> {
1820

@@ -38,17 +40,29 @@ internal abstract class DataCollectorBase<T>(initCapacity: Int) : DataCollector<
3840
data.add(value)
3941
}
4042

41-
protected fun createColumn(name: String, type: KType): DataColumn<T> {
42-
val classifier = type.classifier as KClass<*>
43-
if (classifier.isSubclassOf(DataFrame::class) && !hasNulls) {
44-
return DataColumn.createFrameColumn(name, data as List<AnyFrame>) as DataColumn<T>
45-
}
46-
if (classifier.isSubclassOf(DataRow::class) && !hasNulls) {
47-
val mergedDf = (data as List<AnyRow>).map { it.toDataFrame() }.concat()
48-
return DataColumn.createColumnGroup(name, mergedDf) as DataColumn<T>
49-
}
50-
return DataColumn.createValueColumn(name, data, type.withNullability(hasNulls)) as DataColumn<T>
51-
}
43+
@Suppress("UNCHECKED_CAST")
44+
protected fun createColumn(name: String, type: KType): DataColumn<T> =
45+
when {
46+
type == nothingType -> {
47+
require(values.isEmpty()) { "Cannot create non-empty DataColumn of type Nothing" }
48+
DataColumn.empty(name)
49+
}
50+
51+
type == nullableNothingType -> {
52+
require(values.all { it == null }) { "Cannot create DataColumn of type Nothing? with non-null values" }
53+
DataColumn.createValueColumn(name, values, nullableNothingType)
54+
}
55+
56+
type.isSubtypeOf(typeOf<AnyFrame?>()) && !hasNulls ->
57+
DataColumn.createFrameColumn(name, data as List<AnyFrame>)
58+
59+
type.isSubtypeOf(typeOf<AnyRow?>()) && !hasNulls -> {
60+
val mergedDf = (data as List<AnyRow>).map { it.toDataFrame() }.concat()
61+
DataColumn.createColumnGroup(name, mergedDf).asDataColumn()
62+
}
63+
64+
else -> DataColumn.createValueColumn(name, data, type.withNullability(hasNulls))
65+
}.cast()
5266
}
5367

5468
internal open class ColumnDataCollector(initCapacity: Int = 0, val typeOf: (KClass<*>) -> KType) :
@@ -65,7 +79,7 @@ internal class TypedColumnDataCollector<T>(initCapacity: Int = 0, val type: KTyp
6579
override fun add(value: T?) {
6680
if (checkTypes && value != null && !value.javaClass.kotlin.isSubclassOf(kclass)) {
6781
throw IllegalArgumentException(
68-
"Can not add value of class ${value.javaClass.kotlin.qualifiedName} to column of type $type. Value = $value",
82+
"Cannot add a value of class ${value.javaClass.kotlin.qualifiedName} to a column of type $type. Value: '$value'.",
6983
)
7084
}
7185
super.add(value)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ internal fun <T> concatImpl(name: String, columns: List<DataColumn<T>>): DataCol
2424
internal fun <T> concatImpl(name: String, columns: List<DataColumn<T>?>, columnSizes: List<Int>): DataColumn<T> {
2525
when (columns.size) {
2626
0 -> return DataColumn.empty(name).cast()
27-
1 -> return columns[0] ?: DataColumn.empty(name).cast()
27+
1 -> return columns.single() ?: DataColumn.empty(name).cast()
2828
}
2929

3030
if (columns.all { it == null || it.isColumnGroup() }) {

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

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,25 @@ internal fun <T, C> DataColumn<C>.updateImpl(
101101
): DataColumn<C> {
102102
val collector = createDataCollector<C>(size, type)
103103
val src = this
104-
if (filter == null) {
105-
df.indices().forEach { rowIndex ->
106-
val row = AddDataRowImpl(rowIndex, df, collector.values)
107-
collector.add(expression(row, src, src[rowIndex]))
108-
}
109-
} else {
110-
df.indices().forEach { rowIndex ->
111-
val row = AddDataRowImpl(rowIndex, df, collector.values)
112-
val currentValue = row[src]
113-
val newValue =
114-
if (filter.invoke(row, currentValue)) expression(row, src, currentValue) else currentValue
115-
collector.add(newValue)
104+
try {
105+
if (filter == null) {
106+
df.indices().forEach { rowIndex ->
107+
val row = AddDataRowImpl(rowIndex, df, collector.values)
108+
collector.add(expression(row, src, src[rowIndex]))
109+
}
110+
} else {
111+
df.indices().forEach { rowIndex ->
112+
val row = AddDataRowImpl(rowIndex, df, collector.values)
113+
val currentValue = row[src]
114+
val newValue =
115+
if (filter.invoke(row, currentValue)) expression(row, src, currentValue) else currentValue
116+
collector.add(newValue)
117+
}
116118
}
119+
return collector.toColumn(src.name).cast()
120+
} catch (e: Throwable) {
121+
throw IllegalStateException("Could not update column '${src.name}': ${e.message}", e)
117122
}
118-
return collector.toColumn(src.name).cast()
119123
}
120124

121125
/**

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import org.jetbrains.kotlinx.dataframe.ColumnsContainer
77
import org.jetbrains.kotlinx.dataframe.ColumnsSelector
88
import org.jetbrains.kotlinx.dataframe.DataColumn
99
import org.jetbrains.kotlinx.dataframe.DataFrame
10-
import org.jetbrains.kotlinx.dataframe.DataRow
1110
import org.jetbrains.kotlinx.dataframe.Selector
1211
import org.jetbrains.kotlinx.dataframe.api.AddDataRow
1312
import org.jetbrains.kotlinx.dataframe.api.AddExpression
@@ -43,9 +42,9 @@ import org.jetbrains.kotlinx.dataframe.index
4342
import org.jetbrains.kotlinx.dataframe.nrow
4443
import org.jetbrains.kotlinx.dataframe.util.CREATE_COLUMN
4544
import org.jetbrains.kotlinx.dataframe.util.GUESS_COLUMN_TYPE
46-
import kotlin.reflect.KClass
4745
import kotlin.reflect.KType
4846
import kotlin.reflect.full.isSubtypeOf
47+
import kotlin.reflect.full.starProjectedType
4948
import kotlin.reflect.full.withNullability
5049
import kotlin.reflect.typeOf
5150

@@ -262,10 +261,10 @@ internal fun <T> createColumnGuessingType(
262261
return { value -> if (value != null && value is Number) converter(value) else value }
263262
}
264263

265-
return when (type.classifier!! as KClass<*>) {
264+
return when (type.classifier?.starProjectedType) {
266265
// guessValueType can only return DataRow if all values are `AnyRow?`
267266
// or allColsMakesColGroup == true, and all values are `AnyCol`
268-
DataRow::class ->
267+
typeOf<AnyRow>() ->
269268
if (allColsMakesColGroup && values.firstOrNull() is AnyCol) {
270269
val df = dataFrameOf(values as Iterable<AnyCol>)
271270
DataColumn.createColumnGroup(name, df)
@@ -276,7 +275,7 @@ internal fun <T> createColumnGuessingType(
276275
DataColumn.createColumnGroup(name, df)
277276
}.asDataColumn().cast()
278277

279-
DataFrame::class -> {
278+
typeOf<AnyFrame>() -> {
280279
val frames = values.map {
281280
when (it) {
282281
null -> DataFrame.empty()
@@ -289,7 +288,7 @@ internal fun <T> createColumnGuessingType(
289288
DataColumn.createFrameColumn(name, frames).asDataColumn().cast()
290289
}
291290

292-
List::class -> {
291+
typeOf<List<*>>() -> {
293292
val nullable = type.isMarkedNullable
294293
var isListOfRows: Boolean? = null
295294
val subType = type.arguments.first().type!! // List<T> -> T

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package org.jetbrains.kotlinx.dataframe.api
22

33
import io.kotest.matchers.shouldBe
4+
import org.jetbrains.kotlinx.dataframe.DataColumn
5+
import org.jetbrains.kotlinx.dataframe.DataFrame
46
import org.junit.Test
57

68
class ConcatTests {
@@ -9,7 +11,7 @@ class ConcatTests {
911
fun `different types`() {
1012
val a by columnOf(1, 2)
1113
val b by columnOf(3.0, null)
12-
a.concat(b) shouldBe columnOf(1, 2, 3.0, null).named("a")
14+
a.concat(b) shouldBe columnOf<Number?>(1, 2, 3.0, null).named("a")
1315
}
1416

1517
@Test
@@ -23,4 +25,28 @@ class ConcatTests {
2325

2426
dfWithCategory.columnNames() shouldBe listOf("value", "type", "category")
2527
}
28+
29+
@Test
30+
fun `concat empty DataFrames no rows`() {
31+
val dfWithSchema = DataFrame.emptyOf<Pair<Int, String>>()
32+
(dfWithSchema concat dfWithSchema).let { concatenated ->
33+
concatenated shouldBe dfWithSchema
34+
concatenated.schema() shouldBe dfWithSchema.schema()
35+
}
36+
37+
val dfNothingCols = dataFrameOf(
38+
"a" to DataColumn.empty(),
39+
"b" to DataColumn.empty(),
40+
)
41+
(dfNothingCols concat dfNothingCols).let { concatenated ->
42+
concatenated shouldBe dfNothingCols
43+
concatenated.schema() shouldBe dfNothingCols.schema()
44+
}
45+
}
46+
47+
@Test
48+
fun `concat empty DataFrames no cols`() {
49+
val dfNoCols = DataFrame.empty(5)
50+
(dfNoCols concat dfNoCols) shouldBe DataFrame.empty(10)
51+
}
2652
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package org.jetbrains.kotlinx.dataframe.api
22

3+
import io.kotest.assertions.throwables.shouldThrow
34
import io.kotest.matchers.shouldBe
5+
import org.jetbrains.kotlinx.dataframe.DataColumn
46
import org.jetbrains.kotlinx.dataframe.DataFrame
57
import org.jetbrains.kotlinx.dataframe.annotations.DataSchema
8+
import org.jetbrains.kotlinx.dataframe.impl.nothingType
9+
import org.jetbrains.kotlinx.dataframe.impl.nullableNothingType
610
import org.jetbrains.kotlinx.dataframe.size
711
import org.junit.Test
812

@@ -79,4 +83,25 @@ class UpdateTests {
7983

8084
df.fillNA(SchemaB::i).with { 42 }
8185
}
86+
87+
@Test
88+
fun `update Nothing columns`() {
89+
val emptyDf = dataFrameOf("a" to DataColumn.empty())
90+
emptyDf["a"].type() shouldBe nothingType
91+
92+
emptyDf.update { "a"<Nothing>() }.with { error("should not happen") }
93+
.schema() shouldBe emptyDf.schema()
94+
95+
val nullFilledDf = dataFrameOf("a" to columnOf(null))
96+
nullFilledDf["a"].type() shouldBe nullableNothingType
97+
98+
// can only update with null
99+
nullFilledDf.update { "a"<Nothing?>() }.with { null }
100+
.schema() shouldBe nullFilledDf.schema()
101+
102+
// or 'Nothing', aka, return early/throw exception
103+
shouldThrow<IllegalStateException> {
104+
nullFilledDf.update { "a"<Nothing?>() }.with { error("Nothing") }
105+
}.cause!!.message shouldBe "Nothing"
106+
}
82107
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1712,7 +1712,7 @@ class DataFrameTests : BaseTest() {
17121712
df.update("name").at(0).with { "ALICE" }
17131713
}
17141714

1715-
@Test(expected = IllegalArgumentException::class)
1715+
@Test(expected = IllegalStateException::class)
17161716
fun `update with wrong type`() {
17171717
typed.update("age").with { "string" }
17181718
}

0 commit comments

Comments
 (0)