Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toDataFrame() column order fix with @ColumnName annotations #1004

Merged
merged 1 commit into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -381,19 +381,37 @@ internal fun KCallable<*>.isGetterLike(): Boolean =
else -> false
}

/** @include [KCallable.getterName] */
internal val KFunction<*>.getterName: String
get() = name
.removePrefix("get")
.removePrefix("is")
.replaceFirstChar { it.lowercase() }

/** @include [KCallable.getterName] */
internal val KProperty<*>.getterName: String
get() = name

/**
* Returns the getter name for this callable.
* The name of the callable is returned with proper getter-trimming if it's a [KFunction].
*/
internal val KCallable<*>.getterName: String
get() = when (this) {
is KFunction<*> -> getterName
is KProperty<*> -> getterName
else -> name
}

/** @include [KCallable.columnName] */
@PublishedApi
internal val KFunction<*>.columnName: String
get() = findAnnotation<ColumnName>()?.name
?: name
.removePrefix("get")
.removePrefix("is")
.replaceFirstChar { it.lowercase() }
get() = findAnnotation<ColumnName>()?.name ?: getterName

/** @include [KCallable.columnName] */
@PublishedApi
internal val KProperty<*>.columnName: String
get() = findAnnotation<ColumnName>()?.name ?: name
get() = findAnnotation<ColumnName>()?.name ?: getterName

/**
* Returns the column name for this callable.
Expand All @@ -405,5 +423,5 @@ internal val KCallable<*>.columnName: String
get() = when (this) {
is KFunction<*> -> columnName
is KProperty<*> -> columnName
else -> findAnnotation<ColumnName>()?.name ?: name
else -> findAnnotation<ColumnName>()?.name ?: getterName
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.jetbrains.kotlinx.dataframe.columns.ValueColumn
import org.jetbrains.kotlinx.dataframe.hasNulls
import org.jetbrains.kotlinx.dataframe.impl.columnName
import org.jetbrains.kotlinx.dataframe.impl.commonType
import org.jetbrains.kotlinx.dataframe.impl.getterName
import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema
import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema
Expand Down Expand Up @@ -192,8 +193,8 @@ internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): L
// else sort the ones in the primary constructor first according to the order in there
// leave the rest at the end in lexicographical order
val (propsInConstructor, propsNotInConstructor) =
lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
lexicographicalColumns.partition { it.getterName in primaryConstructorOrder.keys }

return propsInConstructor
.sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
.sortedBy { primaryConstructorOrder[it.getterName] } + propsNotInConstructor
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.kotest.matchers.shouldBe
import org.jetbrains.kotlinx.dataframe.DataColumn
import org.jetbrains.kotlinx.dataframe.DataFrame
import org.jetbrains.kotlinx.dataframe.DataRow
import org.jetbrains.kotlinx.dataframe.annotations.ColumnName
import org.jetbrains.kotlinx.dataframe.annotations.DataSchema
import org.jetbrains.kotlinx.dataframe.columns.ColumnKind
import org.jetbrains.kotlinx.dataframe.kind
Expand Down Expand Up @@ -79,14 +80,35 @@ class CreateDataFrameTests {

@Test
fun `preserve fields order`() {
// constructor properties will keep order, so x, c
class B(val x: Int, val c: String, d: Double) {
// then child properties will be sorted lexicographically by column name, so a, b
val b: Int = x
val a: Double = d
}

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

@Test
fun `preserve fields order with @ColumnName`() {
// constructor properties will keep order, so z, y
class B(
@ColumnName("z") val x: Int,
@ColumnName("y") val c: String,
d: Double,
) {
// then child properties will be sorted lexicographically by column name, so w, x
@ColumnName("x")
val a: Double = d

@ColumnName("w")
val b: Int = x
}

listOf(B(1, "a", 2.0)).toDataFrame().columnNames() shouldBe listOf("z", "y", "w", "x")
}

@DataSchema
data class A(val v: Int)

Expand Down
Loading