diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 43fbc3c60..d005b95dd 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -122,7 +122,7 @@ public interface TraversePropertiesDsl { /** * Skip given [properties] during recursive (dfs) traversal. - * These can also be getter-like functions. + * These can also be getter-like functions (like `getX()` or `isX()`). */ public fun exclude(vararg properties: KCallable<*>) @@ -133,7 +133,7 @@ public interface TraversePropertiesDsl { /** * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns. - * These can also be getter-like functions. + * These can also be getter-like functions (like `getX()` or `isX()`). */ public fun preserve(vararg properties: KCallable<*>) } diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt index 1051712ef..852e724ef 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt @@ -17,6 +17,7 @@ import java.math.BigInteger import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KFunction +import kotlin.reflect.KProperty import kotlin.reflect.KType import kotlin.reflect.KTypeProjection import kotlin.reflect.KVariance @@ -24,6 +25,7 @@ import kotlin.reflect.full.createType import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.full.starProjectedType +import kotlin.reflect.full.valueParameters import kotlin.reflect.full.withNullability import kotlin.reflect.jvm.jvmErasure import kotlin.reflect.typeOf @@ -338,8 +340,22 @@ internal fun List.joinToCamelCaseString(): String { .replaceFirstChar { it.lowercaseChar() } } +internal fun KFunction<*>.isGetterLike(): Boolean = + (name.startsWith("get") || name.startsWith("is")) && valueParameters.isEmpty() + +internal fun KCallable<*>.isGetterLike(): Boolean = + when (this) { + is KProperty<*> -> true + is KFunction<*> -> isGetterLike() + else -> false + } + @PublishedApi -internal val KCallable.columnName: String +internal val KProperty<*>.columnName: String + get() = findAnnotation()?.name ?: name + +@PublishedApi +internal val KCallable<*>.columnName: String get() = findAnnotation()?.name ?: when (this) { // for defining the column names based on a getter-function, we use the function name minus the get/is prefix @@ -349,5 +365,7 @@ internal val KCallable.columnName: String .removePrefix("is") .replaceFirstChar { it.lowercase() } + is KProperty<*> -> this.columnName + else -> name } diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt index ee1a3ff42..183166172 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt @@ -18,9 +18,9 @@ import org.jetbrains.kotlinx.dataframe.impl.asList import org.jetbrains.kotlinx.dataframe.impl.columnName import org.jetbrains.kotlinx.dataframe.impl.emptyPath import org.jetbrains.kotlinx.dataframe.impl.getListType +import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromAnyConstructor -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor +import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal @@ -32,7 +32,6 @@ import kotlin.reflect.full.isSubclassOf import kotlin.reflect.full.memberFunctions import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor -import kotlin.reflect.full.valueParameters import kotlin.reflect.full.withNullability import kotlin.reflect.jvm.isAccessible import kotlin.reflect.jvm.javaField @@ -93,6 +92,11 @@ internal class CreateDataFrameDslImpl( } override fun exclude(vararg properties: KCallable<*>) { + for (prop in properties) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be excluded." + } + } excludeProperties.addAll(properties) } @@ -105,11 +109,22 @@ internal class CreateDataFrameDslImpl( } override fun preserve(vararg properties: KCallable<*>) { + for (prop in properties) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be preserved." + } + } preserveProperties.addAll(properties) } } override fun properties(vararg roots: KCallable<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { + for (prop in roots) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be added as roots." + } + } + val dsl = configuration.clone() if (body != null) { body(dsl) @@ -149,41 +164,20 @@ internal fun convertToDataFrame( preserveProperties: Set>, maxDepth: Int, ): AnyFrame { - val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(clazz) - val anyConstructorOrder = getPropertyOrderFromAnyConstructor(clazz) - val properties: List> = roots .ifEmpty { clazz.memberProperties - .filter { it.visibility == KVisibility.PUBLIC && it.valueParameters.isEmpty() } + .filter { it.visibility == KVisibility.PUBLIC } } - // fall back to getter functions for pojo-like classes + // fall back to getter functions for pojo-like classes if no member properties were found .ifEmpty { clazz.memberFunctions - .filter { - it.visibility == KVisibility.PUBLIC && - it.valueParameters.isEmpty() && - (it.name.startsWith("get") || it.name.startsWith("is")) - } + .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() } } - // sort properties by order of primary-ish constructor - .let { - val names = it.map { it.columnName }.toSet() - - // use the primary constructor order if it's available, - // else try to find a constructor that matches all properties - val order = primaryConstructorOrder - ?: anyConstructorOrder.firstOrNull { map -> - names.all { it in map.keys } - } - if (order != null) { - it.sortedBy { order[it.columnName] ?: Int.MAX_VALUE } - } else { - it - } - } + // sort properties by order in constructors + .sortWithConstructors(clazz) val columns = properties.mapNotNull { val property = it diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt index 02f8e5f19..9400e02b1 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt @@ -13,10 +13,13 @@ import org.jetbrains.kotlinx.dataframe.columns.ColumnKind import org.jetbrains.kotlinx.dataframe.columns.FrameColumn 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.isGetterLike import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema import org.jetbrains.kotlinx.dataframe.type +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.full.primaryConstructor import kotlin.reflect.full.withNullability @@ -148,7 +151,7 @@ internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map v to i } ?.toMap() -internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List> = +internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List> = clazz.constructors .map { constructor -> constructor.parameters @@ -156,3 +159,38 @@ internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List v to i } .toMap() } + +/** + * Sorts [this] according to the order of them in the constructors of [klass]. + * It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting. + * Finally, it falls back to lexicographical sorting if a property does not exist in any constructor. + */ +internal fun Iterable>.sortWithConstructors(klass: KClass<*>): List> { + require(all { it.isGetterLike() }) + val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass) + val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass) + + // starting off lexicographically, sort properties according to the order of all constructors + val allConstructorsSortedProperties = allConstructorsOrders + .fold(this.sortedBy { it.columnName }) { props, constructorOrder -> + props + .withIndex() + .sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i } + .map { it.value } + }.toList() + + if (primaryConstructorOrder == null) { + return allConstructorsSortedProperties + } + + // prefer to sort properties according to the order in the primary constructor if it exists. + // if a property does not exist in the primary constructor, fall back to the other order + + val (propsInConstructor, propsNotInConstructor) = + this.partition { it.columnName in primaryConstructorOrder.keys } + + val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName } + + return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } + + propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) } +} diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt index c071955e0..3d79022a9 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt @@ -24,3 +24,4 @@ fun > T.alsoDebug(println: String? = null, rowsLimit: Int = 20) print(borders = true, title = true, columnTypes = true, valueLimit = -1, rowsLimit = rowsLimit) schema().print() } + 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 index 43fbc3c60..d005b95dd 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -122,7 +122,7 @@ public interface TraversePropertiesDsl { /** * Skip given [properties] during recursive (dfs) traversal. - * These can also be getter-like functions. + * These can also be getter-like functions (like `getX()` or `isX()`). */ public fun exclude(vararg properties: KCallable<*>) @@ -133,7 +133,7 @@ public interface TraversePropertiesDsl { /** * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns. - * These can also be getter-like functions. + * These can also be getter-like functions (like `getX()` or `isX()`). */ public fun preserve(vararg properties: KCallable<*>) } diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt index 1051712ef..852e724ef 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt @@ -17,6 +17,7 @@ import java.math.BigInteger import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KFunction +import kotlin.reflect.KProperty import kotlin.reflect.KType import kotlin.reflect.KTypeProjection import kotlin.reflect.KVariance @@ -24,6 +25,7 @@ import kotlin.reflect.full.createType import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.full.starProjectedType +import kotlin.reflect.full.valueParameters import kotlin.reflect.full.withNullability import kotlin.reflect.jvm.jvmErasure import kotlin.reflect.typeOf @@ -338,8 +340,22 @@ internal fun List.joinToCamelCaseString(): String { .replaceFirstChar { it.lowercaseChar() } } +internal fun KFunction<*>.isGetterLike(): Boolean = + (name.startsWith("get") || name.startsWith("is")) && valueParameters.isEmpty() + +internal fun KCallable<*>.isGetterLike(): Boolean = + when (this) { + is KProperty<*> -> true + is KFunction<*> -> isGetterLike() + else -> false + } + @PublishedApi -internal val KCallable.columnName: String +internal val KProperty<*>.columnName: String + get() = findAnnotation()?.name ?: name + +@PublishedApi +internal val KCallable<*>.columnName: String get() = findAnnotation()?.name ?: when (this) { // for defining the column names based on a getter-function, we use the function name minus the get/is prefix @@ -349,5 +365,7 @@ internal val KCallable.columnName: String .removePrefix("is") .replaceFirstChar { it.lowercase() } + is KProperty<*> -> this.columnName + else -> name } 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 index ee1a3ff42..183166172 100644 --- 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 @@ -18,9 +18,9 @@ import org.jetbrains.kotlinx.dataframe.impl.asList import org.jetbrains.kotlinx.dataframe.impl.columnName import org.jetbrains.kotlinx.dataframe.impl.emptyPath import org.jetbrains.kotlinx.dataframe.impl.getListType +import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromAnyConstructor -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor +import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal @@ -32,7 +32,6 @@ import kotlin.reflect.full.isSubclassOf import kotlin.reflect.full.memberFunctions import kotlin.reflect.full.memberProperties import kotlin.reflect.full.primaryConstructor -import kotlin.reflect.full.valueParameters import kotlin.reflect.full.withNullability import kotlin.reflect.jvm.isAccessible import kotlin.reflect.jvm.javaField @@ -93,6 +92,11 @@ internal class CreateDataFrameDslImpl( } override fun exclude(vararg properties: KCallable<*>) { + for (prop in properties) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be excluded." + } + } excludeProperties.addAll(properties) } @@ -105,11 +109,22 @@ internal class CreateDataFrameDslImpl( } override fun preserve(vararg properties: KCallable<*>) { + for (prop in properties) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be preserved." + } + } preserveProperties.addAll(properties) } } override fun properties(vararg roots: KCallable<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { + for (prop in roots) { + require(prop.isGetterLike()) { + "${prop.name} is not a property or getter-like function. Only those are traversed and can be added as roots." + } + } + val dsl = configuration.clone() if (body != null) { body(dsl) @@ -149,41 +164,20 @@ internal fun convertToDataFrame( preserveProperties: Set>, maxDepth: Int, ): AnyFrame { - val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(clazz) - val anyConstructorOrder = getPropertyOrderFromAnyConstructor(clazz) - val properties: List> = roots .ifEmpty { clazz.memberProperties - .filter { it.visibility == KVisibility.PUBLIC && it.valueParameters.isEmpty() } + .filter { it.visibility == KVisibility.PUBLIC } } - // fall back to getter functions for pojo-like classes + // fall back to getter functions for pojo-like classes if no member properties were found .ifEmpty { clazz.memberFunctions - .filter { - it.visibility == KVisibility.PUBLIC && - it.valueParameters.isEmpty() && - (it.name.startsWith("get") || it.name.startsWith("is")) - } + .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() } } - // sort properties by order of primary-ish constructor - .let { - val names = it.map { it.columnName }.toSet() - - // use the primary constructor order if it's available, - // else try to find a constructor that matches all properties - val order = primaryConstructorOrder - ?: anyConstructorOrder.firstOrNull { map -> - names.all { it in map.keys } - } - if (order != null) { - it.sortedBy { order[it.columnName] ?: Int.MAX_VALUE } - } else { - it - } - } + // sort properties by order in constructors + .sortWithConstructors(clazz) val columns = properties.mapNotNull { val property = it diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt index 02f8e5f19..9400e02b1 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt @@ -13,10 +13,13 @@ import org.jetbrains.kotlinx.dataframe.columns.ColumnKind import org.jetbrains.kotlinx.dataframe.columns.FrameColumn 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.isGetterLike import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema import org.jetbrains.kotlinx.dataframe.type +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.full.primaryConstructor import kotlin.reflect.full.withNullability @@ -148,7 +151,7 @@ internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map v to i } ?.toMap() -internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List> = +internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List> = clazz.constructors .map { constructor -> constructor.parameters @@ -156,3 +159,38 @@ internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List v to i } .toMap() } + +/** + * Sorts [this] according to the order of them in the constructors of [klass]. + * It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting. + * Finally, it falls back to lexicographical sorting if a property does not exist in any constructor. + */ +internal fun Iterable>.sortWithConstructors(klass: KClass<*>): List> { + require(all { it.isGetterLike() }) + val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass) + val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass) + + // starting off lexicographically, sort properties according to the order of all constructors + val allConstructorsSortedProperties = allConstructorsOrders + .fold(this.sortedBy { it.columnName }) { props, constructorOrder -> + props + .withIndex() + .sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i } + .map { it.value } + }.toList() + + if (primaryConstructorOrder == null) { + return allConstructorsSortedProperties + } + + // prefer to sort properties according to the order in the primary constructor if it exists. + // if a property does not exist in the primary constructor, fall back to the other order + + val (propsInConstructor, propsNotInConstructor) = + this.partition { it.columnName in primaryConstructorOrder.keys } + + val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName } + + return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } + + propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) } +} diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt index c071955e0..3d79022a9 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt @@ -24,3 +24,4 @@ fun > T.alsoDebug(println: String? = null, rowsLimit: Int = 20) print(borders = true, title = true, columnTypes = true, valueLimit = -1, rowsLimit = rowsLimit) schema().print() } +