renamed DataColumn.createX() functions and handled other feedback

Jolanrensen · Jolanrensen · commit 1181d5738466 · 2024-10-30T14:40:57.000+01:00
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/DataColumn.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/DataColumn.kt
@@ -133,7 +133,7 @@ public interface DataColumn<out T> : BaseColumn<T> {
          * Creates either a [FrameColumn], [ColumnGroup], or [ValueColumn] by analyzing each value in
          * [values].
          *
-         * This is safer but less efficient than the other functions.
+         * This is safer but slower than the other functions.
          *
          * Some conversions are done automatically to attempt to unify the values.
          *
@@ -151,7 +151,7 @@ public interface DataColumn<out T> : BaseColumn<T> {
          *   See [TypeSuggestion] for more information.
          * @param nullable optionally you can specify whether [values] contains nulls, if `null` it is inferred.
          */
-        public fun <T> createWithTypeInference(
+        public fun <T> createByInference(
             name: String,
             values: List<T>,
             suggestedType: TypeSuggestion = TypeSuggestion.Infer,
@@ -168,7 +168,7 @@ public interface DataColumn<out T> : BaseColumn<T> {
          * Calls [createColumnGroup], [createFrameColumn], or [createValueColumn] based on
          * [type].
          *
-         * This may be unsafe but is more efficient than [createWithTypeInference].
+         * This may be unsafe but is more efficient than [createByInference].
          *
          * Be careful; Values in [values] are NOT checked to adhere to the given [type], nor
          * do we check whether there are unexpected nulls among the values.
@@ -180,7 +180,7 @@ public interface DataColumn<out T> : BaseColumn<T> {
          * @param type the (unchecked) common type of [values]
          * @param infer in case a [ValueColumn] is created, this controls how/whether types need to be inferred
          */
-        public fun <T> createUnsafe(
+        public fun <T> createByType(
             name: String,
             values: List<T>,
             type: KType,
@@ -198,7 +198,8 @@ public interface DataColumn<out T> : BaseColumn<T> {
          * Calls [createColumnGroup], [createFrameColumn], or [createValueColumn] based on
          * type [T].
          *
-         * This is generally safe, as [T] can be inferred, and more efficient than [createWithTypeInference].
+         * This is generally safe, as [T] can be inferred by the compiler,
+         * and more efficient than [createByInference].
          *
          * Be careful when casting occurs; Values in [values] are NOT checked to adhere to the given/inferred type [T],
          * nor do we check whether there are unexpected nulls among the values.
@@ -210,11 +211,11 @@ public interface DataColumn<out T> : BaseColumn<T> {
          * @param values the values to represent each row in the column
          * @param infer in case a [ValueColumn] is created, this controls how/whether types need to be inferred
          */
-        public inline fun <reified T> createUnsafe(
+        public inline fun <reified T> createByType(
             name: String,
             values: List<T>,
             infer: Infer = Infer.None,
-        ): DataColumn<T> = createUnsafe(name, values, typeOf<T>(), infer)
+        ): DataColumn<T> = createByType(name, values, typeOf<T>(), infer)
 
         /** Creates an empty [DataColumn] with given [name]. */
         public fun empty(name: String = ""): AnyCol = createValueColumn(name, emptyList<Unit>(), typeOf<Unit>())
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/constructors.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/constructors.kt
@@ -301,7 +301,7 @@ public fun dataFrameOf(header: Iterable<String>, values: Iterable<Any?>): DataFr
 
 public inline fun <T, reified C> dataFrameOf(header: Iterable<T>, fill: (T) -> Iterable<C>): DataFrame<*> =
     header.map { value ->
-        DataColumn.createWithTypeInference(
+        DataColumn.createByInference(
             name = value.toString(),
             values = fill(value).asList(),
             suggestedType = TypeSuggestion.InferWithUpperbound(typeOf<C>()),
@@ -329,7 +329,7 @@ public class DataFrameBuilder(private val header: List<String>) {
     @JvmName("invoke1")
     internal fun withValues(values: Iterable<Any?>): DataFrame<*> =
         withValuesImpl(header, values.asList()).map { (name, values) ->
-            DataColumn.createWithTypeInference(name, values)
+            DataColumn.createByInference(name, values)
         }.toDataFrame()
 
     public operator fun invoke(args: Sequence<Any?>): DataFrame<*> = invoke(*args.toList().toTypedArray())
@@ -341,7 +341,7 @@ public class DataFrameBuilder(private val header: List<String>) {
 
     public inline operator fun <reified T> invoke(crossinline valuesBuilder: (String) -> Iterable<T>): DataFrame<*> =
         withColumns { name ->
-            DataColumn.createWithTypeInference(
+            DataColumn.createByInference(
                 name = name,
                 values = valuesBuilder(name).asList(),
                 suggestedType = TypeSuggestion.InferWithUpperbound(typeOf<T>()),
@@ -370,15 +370,15 @@ public class DataFrameBuilder(private val header: List<String>) {
 
     public inline fun <reified C> fillIndexed(nrow: Int, crossinline init: (Int, String) -> C): DataFrame<*> =
         withColumns { name ->
-            DataColumn.createWithTypeInference(
+            DataColumn.createByInference(
                 name = name,
                 values = List(nrow) { init(it, name) },
             )
         }
 
     public inline fun <reified C> fill(nrow: Int, crossinline init: (Int) -> C): DataFrame<*> =
         withColumns { name ->
-            DataColumn.createWithTypeInference(
+            DataColumn.createByInference(
                 name = name,
                 values = List(nrow, init),
             )
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/map.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/map.kt
@@ -34,20 +34,20 @@ public inline fun <T, reified R> DataColumn<T>.map(
     crossinline transform: (T) -> R,
 ): DataColumn<R> {
     val newValues = Array(size()) { transform(get(it)) }.asList()
-    return DataColumn.createUnsafe(name(), newValues, typeOf<R>(), infer)
+    return DataColumn.createByType(name(), newValues, typeOf<R>(), infer)
 }
 
 public fun <T, R> DataColumn<T>.map(type: KType, infer: Infer = Infer.Nulls, transform: (T) -> R): DataColumn<R> {
     val values = Array<Any?>(size()) { transform(get(it)) }.asList()
-    return DataColumn.createUnsafe(name(), values, type, infer).cast()
+    return DataColumn.createByType(name(), values, type, infer).cast()
 }
 
 public inline fun <T, reified R> DataColumn<T>.mapIndexed(
     infer: Infer = Infer.Nulls,
     crossinline transform: (Int, T) -> R,
 ): DataColumn<R> {
     val newValues = Array(size()) { transform(it, get(it)) }.asList()
-    return DataColumn.createUnsafe(name(), newValues, typeOf<R>(), infer)
+    return DataColumn.createByType(name(), newValues, typeOf<R>(), infer)
 }
 
 public fun <T, R> DataColumn<T>.mapIndexed(
@@ -56,7 +56,7 @@ public fun <T, R> DataColumn<T>.mapIndexed(
     transform: (Int, T) -> R,
 ): DataColumn<R> {
     val values = Array<Any?>(size()) { transform(it, get(it)) }.asList()
-    return DataColumn.createUnsafe(name(), values, type, infer).cast()
+    return DataColumn.createByType(name(), values, type, infer).cast()
 }
 
 // endregion
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/sort.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/sort.kt
@@ -94,7 +94,7 @@ private interface CommonDataColumnSortWithDocs
 
 /** @include [CommonDataColumnSortWithDocs] */
 public fun <T, C : DataColumn<T>> C.sortWith(comparator: Comparator<T>): C =
-    DataColumn.createUnsafe(name, values().sortedWith(comparator), type) as C
+    DataColumn.createByType(name, values().sortedWith(comparator), type) as C
 
 /** @include [CommonDataColumnSortWithDocs] */
 public fun <T, C : DataColumn<T>> C.sortWith(comparator: (T, T) -> Int): C = sortWith(Comparator(comparator))
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
@@ -190,10 +190,10 @@ public abstract class CreateDataFrameDsl<T> : TraversePropertiesDsl {
         add(columnName, expression)
 
     public inline infix fun <reified R> String.from(inferType: InferType<T, R>): Unit =
-        add(DataColumn.createWithTypeInference(this, source.map { inferType.expression(it) }))
+        add(DataColumn.createByInference(this, source.map { inferType.expression(it) }))
 
     public inline infix fun <reified R> KProperty<R>.from(inferType: InferType<T, R>): Unit =
-        add(DataColumn.createWithTypeInference(columnName, source.map { inferType.expression(it) }))
+        add(DataColumn.createByInference(columnName, source.map { inferType.expression(it) }))
 
     public data class InferType<T, R>(val expression: (T) -> R)
 
@@ -317,13 +317,13 @@ public interface ValueProperty<T> {
 
 public fun Map<String, Iterable<Any?>>.toDataFrame(): AnyFrame =
     map {
-        DataColumn.createWithTypeInference(it.key, it.value.asList())
+        DataColumn.createByInference(it.key, it.value.asList())
     }.toDataFrame()
 
 @JvmName("toDataFrameColumnPathAnyNullable")
 public fun Map<ColumnPath, Iterable<Any?>>.toDataFrame(): AnyFrame =
     map {
-        it.key to DataColumn.createWithTypeInference(
+        it.key to DataColumn.createByInference(
             name = it.key.last(),
             values = it.value.asList(),
         )
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/transpose.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/transpose.kt
@@ -14,7 +14,7 @@ import kotlin.reflect.typeOf
 // region DataRow
 
 public fun <T> DataRow<T>.transpose(): DataFrame<NameValuePair<*>> {
-    val valueColumn = DataColumn.createWithTypeInference(NameValuePair<*>::value.columnName, values)
+    val valueColumn = DataColumn.createByInference(NameValuePair<*>::value.columnName, values)
     val nameColumn = owner.columnNames().toValueColumn(NameValuePair<*>::name)
     return dataFrameOf(nameColumn, valueColumn).cast()
 }
@@ -24,7 +24,7 @@ public inline fun <reified T> AnyRow.transposeTo(): DataFrame<NameValuePair<T>>
 @PublishedApi
 internal fun <T> AnyRow.transposeTo(type: KType): DataFrame<NameValuePair<T>> {
     val convertedValues = values.map { it?.convertTo(type) as T? }
-    val valueColumn = DataColumn.createWithTypeInference(NameValuePair<T>::value.columnName, convertedValues)
+    val valueColumn = DataColumn.createByInference(NameValuePair<T>::value.columnName, convertedValues)
     val nameColumn = owner.columnNames().toValueColumn(NameValuePair<T>::name)
     return dataFrameOf(nameColumn, valueColumn).cast()
 }
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/typeConversions.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/typeConversions.kt
@@ -314,19 +314,19 @@ public fun NullabilityOptions.applyNullability(data: List<Any?>, expectedNulls:
 
 public inline fun <reified T> Iterable<T>.toColumn(name: String = "", infer: Infer = Infer.Nulls): DataColumn<T> =
     if (infer == Infer.Type) {
-        DataColumn.createWithTypeInference(name, asList())
+        DataColumn.createByInference(name, asList())
     } else {
-        DataColumn.createUnsafe(name, asList(), typeOf<T>(), infer)
+        DataColumn.createByType(name, asList(), typeOf<T>(), infer)
     }.forceResolve()
 
 public inline fun <reified T> Iterable<*>.toColumnOf(name: String = ""): DataColumn<T> =
-    DataColumn.createUnsafe(name, asList() as List<T>, typeOf<T>()).forceResolve()
+    DataColumn.createByType(name, asList() as List<T>, typeOf<T>()).forceResolve()
 
 public inline fun <reified T> Iterable<T>.toColumn(ref: ColumnReference<T>): DataColumn<T> =
-    DataColumn.createUnsafe(ref.name(), asList()).forceResolve()
+    DataColumn.createByType(ref.name(), asList()).forceResolve()
 
 public inline fun <reified T> Iterable<T>.toColumn(property: KProperty<T>): DataColumn<T> =
-    DataColumn.createUnsafe(property.columnName, asList()).forceResolve()
+    DataColumn.createByType(property.columnName, asList()).forceResolve()
 
 public fun Iterable<String>.toPath(): ColumnPath = ColumnPath(asList())
 
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/valueCounts.kt
@@ -40,9 +40,9 @@ public fun <T> DataColumn<T>.valueCounts(
     }
     if (dropNA) grouped = grouped.filter { !it.first.isNA }
     val nulls = if (dropNA) false else hasNulls()
-    val values = DataColumn.createUnsafe(name(), grouped.map { it.first }, type().withNullability(nulls))
+    val values = DataColumn.createByType(name(), grouped.map { it.first }, type().withNullability(nulls))
     val countName = if (resultColumn == name()) resultColumn + "1" else resultColumn
-    val counts = DataColumn.createUnsafe(countName, grouped.map { it.second }, typeOf<Int>())
+    val counts = DataColumn.createByType(countName, grouped.map { it.second }, typeOf<Int>())
     return dataFrameOf(values, counts).cast()
 }
 
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/joinWith.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/joinWith.kt
@@ -92,7 +92,7 @@ internal fun <A, B> DataFrame<A>.joinWithImpl(
     }
 
     val df: DataFrame<*> = outputData.mapIndexed { index, values ->
-        DataColumn.createWithTypeInference(generator.names[index], values)
+        DataColumn.createByInference(generator.names[index], values)
     }.toDataFrame()
 
     return df.cast()
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/parse.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/parse.kt
@@ -529,7 +529,7 @@ internal fun DataColumn<String?>.tryParseImpl(options: ParserOptions?): DataColu
 
     // Create a new column with the parsed values,
     // createColumnGuessingType is used to handle unifying values if needed
-    return DataColumn.createWithTypeInference(
+    return DataColumn.createByInference(
         name = name(),
         values = parsedValues,
         suggestedType = TypeSuggestion.Use(type),
@@ -543,7 +543,7 @@ internal fun <T> DataColumn<String?>.parse(parser: StringParser<T>, options: Par
             handler(it.trim()) ?: throw IllegalStateException("Couldn't parse '$it' into type ${parser.type}")
         }
     }
-    return DataColumn.createWithTypeInference(
+    return DataColumn.createByInference(
         name = name(),
         values = parsedValues,
         suggestedType = TypeSuggestion.Use(parser.type.withNullability(hasNulls)),
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
@@ -263,7 +263,7 @@ internal fun convertToDataFrame(
         val shouldCreateColumnGroup = kClass == DataRow::class
 
         when {
-            hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable = nullable)
+            hasExceptions -> DataColumn.createByInference(it.columnName, values, nullable = nullable)
 
             shouldCreateValueCol ->
                 DataColumn.createValueColumn(
@@ -301,7 +301,7 @@ internal fun convertToDataFrame(
                                     (it as? Iterable<*>)?.asList()
                                 }
 
-                                DataColumn.createWithTypeInference(it.columnName, listValues)
+                                DataColumn.createByInference(it.columnName, listValues)
                             }
 
                             elementClass.isValueType -> {
diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/columns/constructors.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/columns/constructors.kt
@@ -61,20 +61,20 @@ internal fun <T, R> ColumnsContainer<T>.newColumn(
     val df = this as? DataFrame<T> ?: dataFrameOf(columns()).cast()
     val (nullable, values) = computeValues(df, expression)
     return when (infer) {
-        Infer.Nulls -> DataColumn.createUnsafe(
+        Infer.Nulls -> DataColumn.createByType(
             name = name,
             values = values,
             type = type.withNullability(nullable).replaceGenericTypeParametersWithUpperbound(),
             infer = Infer.None,
         )
 
-        Infer.Type -> DataColumn.createWithTypeInference(
+        Infer.Type -> DataColumn.createByInference(
             name = name,
             values = values,
             nullable = nullable,
         )
 
-        Infer.None -> DataColumn.createUnsafe(
+        Infer.None -> DataColumn.createByType(
             name = name,
             values = values,
             type = type.replaceGenericTypeParametersWithUpperbound(),
diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/columns/DataColumns.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/columns/DataColumns.kt
@@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.dataframe.columns
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.matchers.shouldBe
 import org.jetbrains.kotlinx.dataframe.AnyFrame
+import org.jetbrains.kotlinx.dataframe.BuildConfig
 import org.jetbrains.kotlinx.dataframe.DataColumn
 import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
 import org.jetbrains.kotlinx.dataframe.api.toColumn
@@ -35,15 +36,16 @@ class DataColumns {
 
     @Test
     fun `allow no nulls in frame columns`() {
-        // enable kotlin.dataframe.debug=true for this
-        shouldThrow<IllegalArgumentException> {
-            DataColumn.createFrameColumn(
-                name = "",
-                groups = listOf(dataFrameOf("a")(1), null) as List<AnyFrame>,
-            )
+        if (BuildConfig.DEBUG) {
+            shouldThrow<IllegalArgumentException> {
+                DataColumn.createFrameColumn(
+                    name = "",
+                    groups = listOf(dataFrameOf("a")(1), null) as List<AnyFrame>,
+                )
+            }
         }
 
-        DataColumn.createUnsafe(
+        DataColumn.createByType(
             name = "",
             values = listOf(dataFrameOf("a")(1), null),
         ).kind() shouldBe ColumnKind.Value
diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/ParserTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/ParserTests.kt
@@ -221,8 +221,9 @@ class ParserTests {
         }
     }
 
+    /** Checks fix for [Issue #593](https://github.com/Kotlin/dataframe/issues/593) */
     @Test
-    fun `Issue #593, mixing null and json`() {
+    fun `Mixing null and json`() {
         val col by columnOf("[\"str\"]", "[]", "null")
         val parsed = col.parse()
         parsed.type() shouldBe typeOf<AnyFrame>()
diff --git a/dataframe-excel/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/xlsx.kt b/dataframe-excel/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/xlsx.kt
@@ -292,7 +292,7 @@ public fun DataFrame.Companion.readExcel(
             val cell: Cell? = row?.getCell(index)
             getCellValue(cell)
         }
-        DataColumn.createWithTypeInference(name, values)
+        DataColumn.createByInference(name, values)
     }
     return dataFrameOf(columns)
 }
diff --git a/plugins/kotlin-dataframe/src/org/jetbrains/kotlinx/dataframe/plugin/impl/DataFrameAdapter.kt b/plugins/kotlin-dataframe/src/org/jetbrains/kotlinx/dataframe/plugin/impl/DataFrameAdapter.kt
@@ -31,7 +31,7 @@ private fun List<SimpleCol>.map(): DataFrame<ConeTypesAdapter> {
 @Suppress("INVISIBLE_REFERENCE")
 fun SimpleCol.asDataColumn(): DataColumn<*> {
     val column = when (this) {
-        is SimpleDataColumn -> DataColumn.createUnsafe(this.name, listOf(this.type))
+        is SimpleDataColumn -> DataColumn.createByType(this.name, listOf(this.type))
         is SimpleColumnGroup -> DataColumn.createColumnGroup(this.name, this.columns().map()) as ColumnGroupImpl<*>
         is SimpleFrameColumn -> DataColumn.createFrameColumn(this.name, listOf(this.columns().map()))
     }

Original file line number	Diff line number	Diff line change
`@@ -40,9 +40,9 @@ public fun <T> DataColumn<T>.valueCounts(`
`40`	`40`	`}`
`41`	`41`	`if (dropNA) grouped = grouped.filter { !it.first.isNA }`
`42`	`42`	`val nulls = if (dropNA) false else hasNulls()`
`43`		`- val values = DataColumn.createUnsafe(name(), grouped.map { it.first }, type().withNullability(nulls))`
	`43`	`+ val values = DataColumn.createByType(name(), grouped.map { it.first }, type().withNullability(nulls))`
`44`	`44`	`val countName = if (resultColumn == name()) resultColumn + "1" else resultColumn`
`45`		`- val counts = DataColumn.createUnsafe(countName, grouped.map { it.second }, typeOf<Int>())`
	`45`	`+ val counts = DataColumn.createByType(countName, grouped.map { it.second }, typeOf<Int>())`
`46`	`46`	`return dataFrameOf(values, counts).cast()`
`47`	`47`	`}`
`48`	`48`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,7 @@ internal fun <A, B> DataFrame<A>.joinWithImpl(`
`92`	`92`	`}`
`93`	`93`
`94`	`94`	`val df: DataFrame<*> = outputData.mapIndexed { index, values ->`
`95`		`- DataColumn.createWithTypeInference(generator.names[index], values)`
	`95`	`+ DataColumn.createByInference(generator.names[index], values)`
`96`	`96`	`}.toDataFrame()`
`97`	`97`
`98`	`98`	`return df.cast()`