Skip to content

Commit a7e4874

Browse files
committed
adding debug mode check for nulls in FrameColumnImpl and simplifying KType.toColumnKind()
1 parent 54d00e6 commit a7e4874

File tree

3 files changed

+44
-8
lines changed

3 files changed

+44
-8
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package org.jetbrains.kotlinx.dataframe.impl.columns
22

33
import org.jetbrains.kotlinx.dataframe.AnyRow
4+
import org.jetbrains.kotlinx.dataframe.BuildConfig
45
import org.jetbrains.kotlinx.dataframe.DataColumn
56
import org.jetbrains.kotlinx.dataframe.DataFrame
67
import org.jetbrains.kotlinx.dataframe.api.schema
78
import org.jetbrains.kotlinx.dataframe.columns.ColumnGroup
89
import org.jetbrains.kotlinx.dataframe.columns.ColumnResolutionContext
910
import org.jetbrains.kotlinx.dataframe.columns.FrameColumn
11+
import org.jetbrains.kotlinx.dataframe.impl.anyNull
1012
import org.jetbrains.kotlinx.dataframe.impl.createStarProjectedType
1113
import org.jetbrains.kotlinx.dataframe.impl.schema.intersectSchemas
1214
import org.jetbrains.kotlinx.dataframe.nrow
@@ -26,6 +28,14 @@ internal open class FrameColumnImpl<T> constructor(
2628
),
2729
FrameColumn<T> {
2830

31+
init {
32+
// Checks for nulls in the `values` list.
33+
// This only runs with `kotlin.dataframe.debug=true` in gradle.properties.
34+
if (BuildConfig.DEBUG) {
35+
require(!values.anyNull()) { "FrameColumn cannot null values." }
36+
}
37+
}
38+
2939
override fun rename(newName: String) = FrameColumnImpl(newName, values, schema, distinct)
3040

3141
override fun defaultValue() = null

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@ package org.jetbrains.kotlinx.dataframe.impl.columns
22

33
import org.jetbrains.kotlinx.dataframe.AnyBaseCol
44
import org.jetbrains.kotlinx.dataframe.AnyCol
5+
import org.jetbrains.kotlinx.dataframe.AnyFrame
6+
import org.jetbrains.kotlinx.dataframe.AnyRow
57
import org.jetbrains.kotlinx.dataframe.ColumnsContainer
68
import org.jetbrains.kotlinx.dataframe.DataColumn
79
import org.jetbrains.kotlinx.dataframe.DataFrame
8-
import org.jetbrains.kotlinx.dataframe.DataRow
910
import org.jetbrains.kotlinx.dataframe.api.asColumnGroup
1011
import org.jetbrains.kotlinx.dataframe.api.cast
1112
import org.jetbrains.kotlinx.dataframe.api.name
@@ -43,7 +44,6 @@ import org.jetbrains.kotlinx.dataframe.nrow
4344
import org.jetbrains.kotlinx.dataframe.type
4445
import kotlin.reflect.KType
4546
import kotlin.reflect.full.isSubtypeOf
46-
import kotlin.reflect.jvm.jvmErasure
4747
import kotlin.reflect.typeOf
4848

4949
internal fun <T> BaseColumn<T>.checkEquals(other: Any?): Boolean {
@@ -471,13 +471,17 @@ internal fun List<ColumnWithPath<*>>.allColumnsExceptKeepingStructure(
471471
return subtrees.map { it.data!!.addPath(it.pathFromRoot()) }
472472
}
473473

474+
/**
475+
* Retrieves the correct [ColumnKind] based on the [type][KType] of the values in the column.
476+
*
477+
* NOTE: nullable DataFrames cannot become a [FrameColumns][FrameColumn],
478+
* so they become [ValueColumns][ValueColumn] instead.
479+
*/
474480
internal fun KType.toColumnKind(): ColumnKind =
475-
jvmErasure.let {
476-
when (it) {
477-
DataFrame::class -> ColumnKind.Frame
478-
DataRow::class -> ColumnKind.Group
479-
else -> ColumnKind.Value
480-
}
481+
when {
482+
this.isSubtypeOf(typeOf<AnyFrame>()) -> ColumnKind.Frame
483+
this.isSubtypeOf(typeOf<AnyRow?>()) -> ColumnKind.Group
484+
else -> ColumnKind.Value
481485
}
482486

483487
internal fun <C> ColumnsResolver<C>.resolve(

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/columns/DataColumns.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package org.jetbrains.kotlinx.dataframe.columns
22

3+
import io.kotest.assertions.throwables.shouldThrow
34
import io.kotest.matchers.shouldBe
5+
import org.jetbrains.kotlinx.dataframe.AnyFrame
6+
import org.jetbrains.kotlinx.dataframe.DataColumn
7+
import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
48
import org.jetbrains.kotlinx.dataframe.api.toColumn
59
import org.jetbrains.kotlinx.dataframe.api.toDataFrame
610
import org.junit.Test
@@ -28,4 +32,22 @@ class DataColumns {
2832
val col = listOf(URI.create("http://example.com"), null).toColumn("a")
2933
col.type().toString() shouldBe "java.net.URI?"
3034
}
35+
36+
@Test
37+
fun `allow no nulls in frame columns`() {
38+
// enable kotlin.dataframe.debug=true for this
39+
shouldThrow<IllegalArgumentException> {
40+
DataColumn.createFrameColumn(
41+
name = "",
42+
groups = listOf(dataFrameOf("a")(1), null) as List<AnyFrame>,
43+
)
44+
}
45+
46+
shouldThrow<IllegalArgumentException> {
47+
DataColumn.create(
48+
"",
49+
listOf(dataFrameOf("a")(1), null),
50+
)
51+
}
52+
}
3153
}

0 commit comments

Comments
 (0)