Skip to content

Commit 505807e

Browse files
authored
Merge pull request #1004 from Kotlin/column-name-order-toDataFrame
`toDataFrame()` column order fix with `@ColumnName` annotations
2 parents 59ebff8 + 5cd3c45 commit 505807e

File tree

3 files changed

+50
-9
lines changed

3 files changed

+50
-9
lines changed

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

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -381,19 +381,37 @@ internal fun KCallable<*>.isGetterLike(): Boolean =
381381
else -> false
382382
}
383383

384+
/** @include [KCallable.getterName] */
385+
internal val KFunction<*>.getterName: String
386+
get() = name
387+
.removePrefix("get")
388+
.removePrefix("is")
389+
.replaceFirstChar { it.lowercase() }
390+
391+
/** @include [KCallable.getterName] */
392+
internal val KProperty<*>.getterName: String
393+
get() = name
394+
395+
/**
396+
* Returns the getter name for this callable.
397+
* The name of the callable is returned with proper getter-trimming if it's a [KFunction].
398+
*/
399+
internal val KCallable<*>.getterName: String
400+
get() = when (this) {
401+
is KFunction<*> -> getterName
402+
is KProperty<*> -> getterName
403+
else -> name
404+
}
405+
384406
/** @include [KCallable.columnName] */
385407
@PublishedApi
386408
internal val KFunction<*>.columnName: String
387-
get() = findAnnotation<ColumnName>()?.name
388-
?: name
389-
.removePrefix("get")
390-
.removePrefix("is")
391-
.replaceFirstChar { it.lowercase() }
409+
get() = findAnnotation<ColumnName>()?.name ?: getterName
392410

393411
/** @include [KCallable.columnName] */
394412
@PublishedApi
395413
internal val KProperty<*>.columnName: String
396-
get() = findAnnotation<ColumnName>()?.name ?: name
414+
get() = findAnnotation<ColumnName>()?.name ?: getterName
397415

398416
/**
399417
* Returns the column name for this callable.
@@ -405,5 +423,5 @@ internal val KCallable<*>.columnName: String
405423
get() = when (this) {
406424
is KFunction<*> -> columnName
407425
is KProperty<*> -> columnName
408-
else -> findAnnotation<ColumnName>()?.name ?: name
426+
else -> findAnnotation<ColumnName>()?.name ?: getterName
409427
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import org.jetbrains.kotlinx.dataframe.columns.ValueColumn
1515
import org.jetbrains.kotlinx.dataframe.hasNulls
1616
import org.jetbrains.kotlinx.dataframe.impl.columnName
1717
import org.jetbrains.kotlinx.dataframe.impl.commonType
18+
import org.jetbrains.kotlinx.dataframe.impl.getterName
1819
import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
1920
import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema
2021
import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema
@@ -192,8 +193,8 @@ internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): L
192193
// else sort the ones in the primary constructor first according to the order in there
193194
// leave the rest at the end in lexicographical order
194195
val (propsInConstructor, propsNotInConstructor) =
195-
lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
196+
lexicographicalColumns.partition { it.getterName in primaryConstructorOrder.keys }
196197

197198
return propsInConstructor
198-
.sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
199+
.sortedBy { primaryConstructorOrder[it.getterName] } + propsNotInConstructor
199200
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import io.kotest.matchers.shouldBe
55
import org.jetbrains.kotlinx.dataframe.DataColumn
66
import org.jetbrains.kotlinx.dataframe.DataFrame
77
import org.jetbrains.kotlinx.dataframe.DataRow
8+
import org.jetbrains.kotlinx.dataframe.annotations.ColumnName
89
import org.jetbrains.kotlinx.dataframe.annotations.DataSchema
910
import org.jetbrains.kotlinx.dataframe.columns.ColumnKind
1011
import org.jetbrains.kotlinx.dataframe.kind
@@ -79,14 +80,35 @@ class CreateDataFrameTests {
7980

8081
@Test
8182
fun `preserve fields order`() {
83+
// constructor properties will keep order, so x, c
8284
class B(val x: Int, val c: String, d: Double) {
85+
// then child properties will be sorted lexicographically by column name, so a, b
8386
val b: Int = x
8487
val a: Double = d
8588
}
8689

8790
listOf(B(1, "a", 2.0)).toDataFrame().columnNames() shouldBe listOf("x", "c", "a", "b")
8891
}
8992

93+
@Test
94+
fun `preserve fields order with @ColumnName`() {
95+
// constructor properties will keep order, so z, y
96+
class B(
97+
@ColumnName("z") val x: Int,
98+
@ColumnName("y") val c: String,
99+
d: Double,
100+
) {
101+
// then child properties will be sorted lexicographically by column name, so w, x
102+
@ColumnName("x")
103+
val a: Double = d
104+
105+
@ColumnName("w")
106+
val b: Int = x
107+
}
108+
109+
listOf(B(1, "a", 2.0)).toDataFrame().columnNames() shouldBe listOf("z", "y", "w", "x")
110+
}
111+
90112
@DataSchema
91113
data class A(val v: Int)
92114

0 commit comments

Comments
 (0)