From 8ff090b9e2c1582dc222e8136c7c67aa9e955fba Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Sat, 6 Apr 2024 14:07:50 +0200 Subject: [PATCH 01/13] adding pojo tests --- .../kotlinx/dataframe/api/JavaPojo.java | 58 ++++++++++++++++++ .../kotlinx/dataframe/api/toDataFrame.kt | 60 +++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java diff --git a/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java new file mode 100644 index 000000000..dddcbd572 --- /dev/null +++ b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -0,0 +1,58 @@ +package org.jetbrains.kotlinx.dataframe.api; + +import java.util.Objects; + +public class JavaPojo { + + private int a; + private String b; + + public JavaPojo() {} + + public JavaPojo(int a, String b) { + this.a = a; + this.b = b; + } + + public int getA() { + return a; + } + + public void setA(int a) { + this.a = a; + } + + public String getB() { + return b; + } + + public void setB(String b) { + this.b = b; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + JavaPojo testPojo = (JavaPojo) o; + + if (a != testPojo.a) return false; + return Objects.equals(b, testPojo.b); + } + + @Override + public int hashCode() { + int result = a; + result = 31 * result + (b != null ? b.hashCode() : 0); + return result; + } + + @Override + public String toString() { + return "TestPojo{" + + "a=" + a + + ", b='" + b + '\'' + + '}'; + } +} diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index f5247ccc6..1eebb0bb7 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -312,4 +312,64 @@ class CreateDataFrameTests { fun `convert private class with public members`() { listOf(PrivateClass(1)).toDataFrame() shouldBe dataFrameOf("a")(1) } + + class KotlinPojo { + + private var a: Int = 0 + private var b: String = "" + + constructor() + + constructor(a: Int, b: String) { + this.a = a + this.b = b + } + + fun getA(): Int = a + fun setA(a: Int) { + this.a = a + } + + fun getB(): String = b + fun setB(b: String) { + this.b = b + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is KotlinPojo) return false + + if (a != other.a) return false + if (b != other.b) return false + + return true + } + + override fun hashCode(): Int { + var result = a + result = 31 * result + b.hashCode() + return result + } + + override fun toString(): String { + return "FakePojo(a=$a, b='$b')" + } + } + + @Test + fun `convert POJO to DF`() { + listOf(KotlinPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + listOf(JavaPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + + listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) + listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("a") + + listOf(JavaPojo(1, "a")).toDataFrame { + properties(JavaPojo::getA) + } shouldBe dataFrameOf("a")(1) + + listOf(JavaPojo(1, "a")).toDataFrame { + properties(JavaPojo::getB) + } shouldBe dataFrameOf("b")("a") + } } From b82d08bce2bc00facb712f768d75917d6243cdf3 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Sat, 6 Apr 2024 14:14:51 +0200 Subject: [PATCH 02/13] relaxing toDataFrame properties DSL to allow for getter functions too. If no memberProperties are available, fall back to getter-like memberFunctions. Also, it now tries to sort by any constructor if there are multiple (common in java with a no-arg constructor) --- .../kotlinx/dataframe/api/toDataFrame.kt | 17 +++-- .../dataframe/codeGen/MarkersExtractor.kt | 4 +- .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 16 ++++- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 65 ++++++++++++++----- .../kotlinx/dataframe/impl/schema/Utils.kt | 17 ++++- 5 files changed, 90 insertions(+), 29 deletions(-) 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 ec3832563..43fbc3c60 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 @@ -13,6 +13,7 @@ import org.jetbrains.kotlinx.dataframe.impl.asList import org.jetbrains.kotlinx.dataframe.impl.columnName import org.jetbrains.kotlinx.dataframe.impl.columns.guessColumnType import org.jetbrains.kotlinx.dataframe.index +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KProperty @@ -115,24 +116,26 @@ public fun Iterable>>.toDataFrameFromPairs(): AnyFra public interface TraversePropertiesDsl { /** - * Skip given [classes] during recursive (dfs) traversal + * Skip given [classes] during recursive (dfs) traversal. */ public fun exclude(vararg classes: KClass<*>) /** - * Skip given [properties] during recursive (dfs) traversal + * Skip given [properties] during recursive (dfs) traversal. + * These can also be getter-like functions. */ - public fun exclude(vararg properties: KProperty<*>) + public fun exclude(vararg properties: KCallable<*>) /** - * Store given [classes] in ValueColumns without transformation into ColumnGroups or FrameColumns + * Store given [classes] in ValueColumns without transformation into ColumnGroups or FrameColumns. */ public fun preserve(vararg classes: KClass<*>) /** - * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns + * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns. + * These can also be getter-like functions. */ - public fun preserve(vararg properties: KProperty<*>) + public fun preserve(vararg properties: KCallable<*>) } public inline fun TraversePropertiesDsl.preserve(): Unit = preserve(T::class) @@ -148,7 +151,7 @@ public abstract class CreateDataFrameDsl : TraversePropertiesDsl { public infix fun AnyBaseCol.into(path: ColumnPath): Unit = add(this, path) public abstract fun properties( - vararg roots: KProperty<*>, + vararg roots: KCallable<*>, maxDepth: Int = 0, body: (TraversePropertiesDsl.() -> Unit)? = null, ) diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt index ef883a33e..833862643 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt @@ -4,7 +4,7 @@ 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.impl.schema.getPropertiesOrder +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema import kotlin.reflect.KClass import kotlin.reflect.KType @@ -53,7 +53,7 @@ internal object MarkersExtractor { } private fun getFields(markerClass: KClass<*>, nullableProperties: Boolean): List { - val order = getPropertiesOrder(markerClass) + val order = getPropertyOrderFromPrimaryConstructor(markerClass) ?: emptyMap() return markerClass.memberProperties.sortedBy { order[it.name] ?: Int.MAX_VALUE }.mapIndexed { _, it -> val fieldName = ValidFieldName.of(it.name) val columnName = it.findAnnotation()?.name ?: fieldName.unquoted 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 6d4b89a8f..1051712ef 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 @@ -14,8 +14,9 @@ import org.jetbrains.kotlinx.dataframe.impl.columns.toColumnSet import org.jetbrains.kotlinx.dataframe.nrow import java.math.BigDecimal import java.math.BigInteger +import kotlin.reflect.KCallable import kotlin.reflect.KClass -import kotlin.reflect.KProperty +import kotlin.reflect.KFunction import kotlin.reflect.KType import kotlin.reflect.KTypeProjection import kotlin.reflect.KVariance @@ -338,4 +339,15 @@ internal fun List.joinToCamelCaseString(): String { } @PublishedApi -internal val KProperty.columnName: String get() = findAnnotation()?.name ?: name +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 + is KFunction<*> -> + name + .removePrefix("get") + .removePrefix("is") + .replaceFirstChar { it.lowercase() } + + 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 56f99abd0..cd59ad05f 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 @@ -19,16 +19,20 @@ 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.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertiesOrder +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromAnyConstructor +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KProperty import kotlin.reflect.KVisibility 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 @@ -72,13 +76,13 @@ internal class CreateDataFrameDslImpl( internal class TraverseConfiguration : TraversePropertiesDsl { - val excludeProperties = mutableSetOf>() + val excludeProperties = mutableSetOf>() val excludeClasses = mutableSetOf>() val preserveClasses = mutableSetOf>() - val preserveProperties = mutableSetOf>() + val preserveProperties = mutableSetOf>() fun clone(): TraverseConfiguration = TraverseConfiguration().also { it.excludeClasses.addAll(excludeClasses) @@ -87,7 +91,7 @@ internal class CreateDataFrameDslImpl( it.preserveClasses.addAll(preserveClasses) } - override fun exclude(vararg properties: KProperty<*>) { + override fun exclude(vararg properties: KCallable<*>) { excludeProperties.addAll(properties) } @@ -99,12 +103,12 @@ internal class CreateDataFrameDslImpl( preserveClasses.addAll(classes) } - override fun preserve(vararg properties: KProperty<*>) { + override fun preserve(vararg properties: KCallable<*>) { preserveProperties.addAll(properties) } } - override fun properties(vararg roots: KProperty<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { + override fun properties(vararg roots: KCallable<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { val dsl = configuration.clone() if (body != null) { body(dsl) @@ -127,18 +131,47 @@ internal fun Iterable.createDataFrameImpl(clazz: KClass<*>, body: CreateD internal fun convertToDataFrame( data: Iterable<*>, clazz: KClass<*>, - roots: List>, - excludes: Set>, + roots: List>, + excludes: Set>, preserveClasses: Set>, - preserveProperties: Set>, - maxDepth: Int + preserveProperties: Set>, + maxDepth: Int, ): AnyFrame { - val order = getPropertiesOrder(clazz) + val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(clazz) + val anyConstructorOrder = getPropertyOrderFromAnyConstructor(clazz) - val properties = roots.ifEmpty { - clazz.memberProperties - .filter { it.visibility == KVisibility.PUBLIC && it.parameters.toList().size == 1 } - }.sortedBy { order[it.name] ?: Int.MAX_VALUE } + val properties: List> = roots + .ifEmpty { + clazz.memberProperties + .filter { it.visibility == KVisibility.PUBLIC && it.valueParameters.isEmpty() } + } + + // fall back to getter functions for pojo-like classes + .ifEmpty { + clazz.memberFunctions + .filter { + it.visibility == KVisibility.PUBLIC && + it.valueParameters.isEmpty() && + (it.name.startsWith("get") || it.name.startsWith("is")) + } + } + + // 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 + } + } val columns = properties.mapNotNull { val property = it @@ -162,7 +195,7 @@ internal fun convertToDataFrame( valueClassConverter } } - property.javaField?.isAccessible = true + (property as? KProperty<*>)?.javaField?.isAccessible = true property.isAccessible = true var nullable = false 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 16acc7aba..02f8e5f19 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 @@ -141,5 +141,18 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame = internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame = MarkersExtractor.get(clazz).schema.createEmptyDataFrame() -internal fun getPropertiesOrder(clazz: KClass<*>): Map = - clazz.primaryConstructor?.parameters?.mapNotNull { it.name }?.mapIndexed { i, v -> v to i }?.toMap() ?: emptyMap() +internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map? = + clazz.primaryConstructor + ?.parameters + ?.mapNotNull { it.name } + ?.mapIndexed { i, v -> v to i } + ?.toMap() + +internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List> = + clazz.constructors + .map { constructor -> + constructor.parameters + .mapNotNull { it.name } + .mapIndexed { i, v -> v to i } + .toMap() + } From 1dd615160999fee42b4e3c1104184a4212cb462b Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Sat, 6 Apr 2024 14:17:26 +0200 Subject: [PATCH 03/13] linting and generating sources --- .../kotlinx/dataframe/api/toDataFrame.kt | 17 +- .../dataframe/codeGen/MarkersExtractor.kt | 4 +- .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 16 +- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 164 +++++++++++++----- .../kotlinx/dataframe/impl/schema/Utils.kt | 17 +- .../kotlinx/dataframe/api/JavaPojo.java | 58 +++++++ .../kotlinx/dataframe/api/toDataFrame.kt | 60 +++++++ .../kotlinx/dataframe/impl/api/toDataFrame.kt | 99 ++++++++--- 8 files changed, 358 insertions(+), 77 deletions(-) create mode 100644 core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java 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 ec3832563..43fbc3c60 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 @@ -13,6 +13,7 @@ import org.jetbrains.kotlinx.dataframe.impl.asList import org.jetbrains.kotlinx.dataframe.impl.columnName import org.jetbrains.kotlinx.dataframe.impl.columns.guessColumnType import org.jetbrains.kotlinx.dataframe.index +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KProperty @@ -115,24 +116,26 @@ public fun Iterable>>.toDataFrameFromPairs(): AnyFra public interface TraversePropertiesDsl { /** - * Skip given [classes] during recursive (dfs) traversal + * Skip given [classes] during recursive (dfs) traversal. */ public fun exclude(vararg classes: KClass<*>) /** - * Skip given [properties] during recursive (dfs) traversal + * Skip given [properties] during recursive (dfs) traversal. + * These can also be getter-like functions. */ - public fun exclude(vararg properties: KProperty<*>) + public fun exclude(vararg properties: KCallable<*>) /** - * Store given [classes] in ValueColumns without transformation into ColumnGroups or FrameColumns + * Store given [classes] in ValueColumns without transformation into ColumnGroups or FrameColumns. */ public fun preserve(vararg classes: KClass<*>) /** - * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns + * Store given [properties] in ValueColumns without transformation into ColumnGroups or FrameColumns. + * These can also be getter-like functions. */ - public fun preserve(vararg properties: KProperty<*>) + public fun preserve(vararg properties: KCallable<*>) } public inline fun TraversePropertiesDsl.preserve(): Unit = preserve(T::class) @@ -148,7 +151,7 @@ public abstract class CreateDataFrameDsl : TraversePropertiesDsl { public infix fun AnyBaseCol.into(path: ColumnPath): Unit = add(this, path) public abstract fun properties( - vararg roots: KProperty<*>, + vararg roots: KCallable<*>, maxDepth: Int = 0, body: (TraversePropertiesDsl.() -> Unit)? = null, ) diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt index ef883a33e..833862643 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/MarkersExtractor.kt @@ -4,7 +4,7 @@ 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.impl.schema.getPropertiesOrder +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema import kotlin.reflect.KClass import kotlin.reflect.KType @@ -53,7 +53,7 @@ internal object MarkersExtractor { } private fun getFields(markerClass: KClass<*>, nullableProperties: Boolean): List { - val order = getPropertiesOrder(markerClass) + val order = getPropertyOrderFromPrimaryConstructor(markerClass) ?: emptyMap() return markerClass.memberProperties.sortedBy { order[it.name] ?: Int.MAX_VALUE }.mapIndexed { _, it -> val fieldName = ValidFieldName.of(it.name) val columnName = it.findAnnotation()?.name ?: fieldName.unquoted 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 6d4b89a8f..1051712ef 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 @@ -14,8 +14,9 @@ import org.jetbrains.kotlinx.dataframe.impl.columns.toColumnSet import org.jetbrains.kotlinx.dataframe.nrow import java.math.BigDecimal import java.math.BigInteger +import kotlin.reflect.KCallable import kotlin.reflect.KClass -import kotlin.reflect.KProperty +import kotlin.reflect.KFunction import kotlin.reflect.KType import kotlin.reflect.KTypeProjection import kotlin.reflect.KVariance @@ -338,4 +339,15 @@ internal fun List.joinToCamelCaseString(): String { } @PublishedApi -internal val KProperty.columnName: String get() = findAnnotation()?.name ?: name +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 + is KFunction<*> -> + name + .removePrefix("get") + .removePrefix("is") + .replaceFirstChar { it.lowercase() } + + 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 56f99abd0..ee1a3ff42 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 @@ -19,16 +19,20 @@ 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.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertiesOrder +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromAnyConstructor +import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertyOrderFromPrimaryConstructor import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal +import kotlin.reflect.KCallable import kotlin.reflect.KClass import kotlin.reflect.KProperty import kotlin.reflect.KVisibility 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 @@ -43,11 +47,12 @@ internal val valueTypes = setOf( kotlinx.datetime.Instant::class, ) -internal val KClass<*>.isValueType: Boolean get() = - this in valueTypes || - this.isSubclassOf(Number::class) || - this.isSubclassOf(Enum::class) || - this.isSubclassOf(Temporal::class) +internal val KClass<*>.isValueType: Boolean + get() = + this in valueTypes || + this.isSubclassOf(Number::class) || + this.isSubclassOf(Enum::class) || + this.isSubclassOf(Temporal::class) internal class CreateDataFrameDslImpl( override val source: Iterable, @@ -72,13 +77,13 @@ internal class CreateDataFrameDslImpl( internal class TraverseConfiguration : TraversePropertiesDsl { - val excludeProperties = mutableSetOf>() + val excludeProperties = mutableSetOf>() val excludeClasses = mutableSetOf>() val preserveClasses = mutableSetOf>() - val preserveProperties = mutableSetOf>() + val preserveProperties = mutableSetOf>() fun clone(): TraverseConfiguration = TraverseConfiguration().also { it.excludeClasses.addAll(excludeClasses) @@ -87,7 +92,7 @@ internal class CreateDataFrameDslImpl( it.preserveClasses.addAll(preserveClasses) } - override fun exclude(vararg properties: KProperty<*>) { + override fun exclude(vararg properties: KCallable<*>) { excludeProperties.addAll(properties) } @@ -99,17 +104,25 @@ internal class CreateDataFrameDslImpl( preserveClasses.addAll(classes) } - override fun preserve(vararg properties: KProperty<*>) { + override fun preserve(vararg properties: KCallable<*>) { preserveProperties.addAll(properties) } } - override fun properties(vararg roots: KProperty<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { + override fun properties(vararg roots: KCallable<*>, maxDepth: Int, body: (TraversePropertiesDsl.() -> Unit)?) { val dsl = configuration.clone() if (body != null) { body(dsl) } - val df = convertToDataFrame(source, clazz, roots.toList(), dsl.excludeProperties, dsl.preserveClasses, dsl.preserveProperties, maxDepth) + val df = convertToDataFrame( + data = source, + clazz = clazz, + roots = roots.toList(), + excludes = dsl.excludeProperties, + preserveClasses = dsl.preserveClasses, + preserveProperties = dsl.preserveProperties, + maxDepth = maxDepth, + ) df.columns().forEach { add(it) } @@ -117,7 +130,10 @@ internal class CreateDataFrameDslImpl( } @PublishedApi -internal fun Iterable.createDataFrameImpl(clazz: KClass<*>, body: CreateDataFrameDslImpl.() -> Unit): DataFrame { +internal fun Iterable.createDataFrameImpl( + clazz: KClass<*>, + body: CreateDataFrameDslImpl.() -> Unit, +): DataFrame { val builder = CreateDataFrameDslImpl(this, clazz) builder.body() return builder.columns.toDataFrameFromPairs() @@ -127,18 +143,47 @@ internal fun Iterable.createDataFrameImpl(clazz: KClass<*>, body: CreateD internal fun convertToDataFrame( data: Iterable<*>, clazz: KClass<*>, - roots: List>, - excludes: Set>, + roots: List>, + excludes: Set>, preserveClasses: Set>, - preserveProperties: Set>, - maxDepth: Int + preserveProperties: Set>, + maxDepth: Int, ): AnyFrame { - val order = getPropertiesOrder(clazz) + val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(clazz) + val anyConstructorOrder = getPropertyOrderFromAnyConstructor(clazz) + + val properties: List> = roots + .ifEmpty { + clazz.memberProperties + .filter { it.visibility == KVisibility.PUBLIC && it.valueParameters.isEmpty() } + } + + // fall back to getter functions for pojo-like classes + .ifEmpty { + clazz.memberFunctions + .filter { + it.visibility == KVisibility.PUBLIC && + it.valueParameters.isEmpty() && + (it.name.startsWith("get") || it.name.startsWith("is")) + } + } + + // sort properties by order of primary-ish constructor + .let { + val names = it.map { it.columnName }.toSet() - val properties = roots.ifEmpty { - clazz.memberProperties - .filter { it.visibility == KVisibility.PUBLIC && it.parameters.toList().size == 1 } - }.sortedBy { order[it.name] ?: Int.MAX_VALUE } + // 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 + } + } val columns = properties.mapNotNull { val property = it @@ -148,8 +193,9 @@ internal fun convertToDataFrame( val valueClassConverter = (it.returnType.classifier as? KClass<*>)?.let { kClass -> if (!kClass.isValue) null else { - val constructor = - requireNotNull(kClass.primaryConstructor) { "value class $kClass is expected to have primary constructor, but couldn't obtain it" } + val constructor = requireNotNull(kClass.primaryConstructor) { + "value class $kClass is expected to have primary constructor, but couldn't obtain it" + } val parameter = constructor.parameters.singleOrNull() ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") // there's no need to unwrap if underlying field is nullable @@ -162,7 +208,7 @@ internal fun convertToDataFrame( valueClassConverter } } - property.javaField?.isAccessible = true + (property as? KProperty<*>)?.javaField?.isAccessible = true property.isAccessible = true var nullable = false @@ -208,18 +254,34 @@ internal fun convertToDataFrame( val kclass = (returnType.classifier as KClass<*>) when { hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable) - kclass == Any::class || preserveClasses.contains(kclass) || preserveProperties.contains(property) || (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || kclass.isValueType -> - DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) - kclass == DataFrame::class && !nullable -> DataColumn.createFrameColumn(it.columnName, values as List) - kclass == DataRow::class -> DataColumn.createColumnGroup(it.columnName, (values as List).concat()) + + kclass == Any::class || + preserveClasses.contains(kclass) || + preserveProperties.contains(property) || + (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || + kclass.isValueType -> + DataColumn.createValueColumn( + name = it.columnName, + values = values, + type = returnType.withNullability(nullable), + ) + + kclass == DataFrame::class && !nullable -> + DataColumn.createFrameColumn( + name = it.columnName, + groups = values as List + ) + + kclass == DataRow::class -> + DataColumn.createColumnGroup( + name = it.columnName, + df = (values as List).concat(), + ) + kclass.isSubclassOf(Iterable::class) -> { val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type if (elementType == null) { - DataColumn.createValueColumn( - it.columnName, - values, - returnType.withNullability(nullable) - ) + DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) } else { val elementClass = (elementType.classifier as? KClass<*>) @@ -231,6 +293,7 @@ internal fun convertToDataFrame( DataColumn.createWithTypeInference(it.columnName, listValues) } + elementClass.isValueType -> { val listType = getListType(elementType).withNullability(nullable) val listValues = values.map { @@ -238,12 +301,22 @@ internal fun convertToDataFrame( } DataColumn.createValueColumn(it.columnName, listValues, listType) } + else -> { val frames = values.map { - if (it == null) DataFrame.empty() - else { + if (it == null) { + DataFrame.empty() + } else { require(it is Iterable<*>) - convertToDataFrame(it, elementClass, emptyList(), excludes, preserveClasses, preserveProperties, maxDepth - 1) + convertToDataFrame( + data = it, + clazz = elementClass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) } } DataColumn.createFrameColumn(it.columnName, frames) @@ -251,13 +324,24 @@ internal fun convertToDataFrame( } } } + else -> { - val df = convertToDataFrame(values, kclass, emptyList(), excludes, preserveClasses, preserveProperties, maxDepth - 1) - DataColumn.createColumnGroup(it.columnName, df) + val df = convertToDataFrame( + data = values, + clazz = kclass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) + DataColumn.createColumnGroup(name = it.columnName, df = df) } } } return if (columns.isEmpty()) { DataFrame.empty(data.count()) - } else dataFrameOf(columns) + } else { + dataFrameOf(columns) + } } 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 16acc7aba..02f8e5f19 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 @@ -141,5 +141,18 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame = internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame = MarkersExtractor.get(clazz).schema.createEmptyDataFrame() -internal fun getPropertiesOrder(clazz: KClass<*>): Map = - clazz.primaryConstructor?.parameters?.mapNotNull { it.name }?.mapIndexed { i, v -> v to i }?.toMap() ?: emptyMap() +internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map? = + clazz.primaryConstructor + ?.parameters + ?.mapNotNull { it.name } + ?.mapIndexed { i, v -> v to i } + ?.toMap() + +internal fun getPropertyOrderFromAnyConstructor(clazz: KClass<*>): List> = + clazz.constructors + .map { constructor -> + constructor.parameters + .mapNotNull { it.name } + .mapIndexed { i, v -> v to i } + .toMap() + } diff --git a/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java new file mode 100644 index 000000000..dddcbd572 --- /dev/null +++ b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -0,0 +1,58 @@ +package org.jetbrains.kotlinx.dataframe.api; + +import java.util.Objects; + +public class JavaPojo { + + private int a; + private String b; + + public JavaPojo() {} + + public JavaPojo(int a, String b) { + this.a = a; + this.b = b; + } + + public int getA() { + return a; + } + + public void setA(int a) { + this.a = a; + } + + public String getB() { + return b; + } + + public void setB(String b) { + this.b = b; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + JavaPojo testPojo = (JavaPojo) o; + + if (a != testPojo.a) return false; + return Objects.equals(b, testPojo.b); + } + + @Override + public int hashCode() { + int result = a; + result = 31 * result + (b != null ? b.hashCode() : 0); + return result; + } + + @Override + public String toString() { + return "TestPojo{" + + "a=" + a + + ", b='" + b + '\'' + + '}'; + } +} diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index f5247ccc6..1eebb0bb7 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -312,4 +312,64 @@ class CreateDataFrameTests { fun `convert private class with public members`() { listOf(PrivateClass(1)).toDataFrame() shouldBe dataFrameOf("a")(1) } + + class KotlinPojo { + + private var a: Int = 0 + private var b: String = "" + + constructor() + + constructor(a: Int, b: String) { + this.a = a + this.b = b + } + + fun getA(): Int = a + fun setA(a: Int) { + this.a = a + } + + fun getB(): String = b + fun setB(b: String) { + this.b = b + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is KotlinPojo) return false + + if (a != other.a) return false + if (b != other.b) return false + + return true + } + + override fun hashCode(): Int { + var result = a + result = 31 * result + b.hashCode() + return result + } + + override fun toString(): String { + return "FakePojo(a=$a, b='$b')" + } + } + + @Test + fun `convert POJO to DF`() { + listOf(KotlinPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + listOf(JavaPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + + listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) + listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("a") + + listOf(JavaPojo(1, "a")).toDataFrame { + properties(JavaPojo::getA) + } shouldBe dataFrameOf("a")(1) + + listOf(JavaPojo(1, "a")).toDataFrame { + properties(JavaPojo::getB) + } shouldBe dataFrameOf("b")("a") + } } 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 cd59ad05f..ee1a3ff42 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 @@ -47,11 +47,12 @@ internal val valueTypes = setOf( kotlinx.datetime.Instant::class, ) -internal val KClass<*>.isValueType: Boolean get() = - this in valueTypes || - this.isSubclassOf(Number::class) || - this.isSubclassOf(Enum::class) || - this.isSubclassOf(Temporal::class) +internal val KClass<*>.isValueType: Boolean + get() = + this in valueTypes || + this.isSubclassOf(Number::class) || + this.isSubclassOf(Enum::class) || + this.isSubclassOf(Temporal::class) internal class CreateDataFrameDslImpl( override val source: Iterable, @@ -113,7 +114,15 @@ internal class CreateDataFrameDslImpl( if (body != null) { body(dsl) } - val df = convertToDataFrame(source, clazz, roots.toList(), dsl.excludeProperties, dsl.preserveClasses, dsl.preserveProperties, maxDepth) + val df = convertToDataFrame( + data = source, + clazz = clazz, + roots = roots.toList(), + excludes = dsl.excludeProperties, + preserveClasses = dsl.preserveClasses, + preserveProperties = dsl.preserveProperties, + maxDepth = maxDepth, + ) df.columns().forEach { add(it) } @@ -121,7 +130,10 @@ internal class CreateDataFrameDslImpl( } @PublishedApi -internal fun Iterable.createDataFrameImpl(clazz: KClass<*>, body: CreateDataFrameDslImpl.() -> Unit): DataFrame { +internal fun Iterable.createDataFrameImpl( + clazz: KClass<*>, + body: CreateDataFrameDslImpl.() -> Unit, +): DataFrame { val builder = CreateDataFrameDslImpl(this, clazz) builder.body() return builder.columns.toDataFrameFromPairs() @@ -181,8 +193,9 @@ internal fun convertToDataFrame( val valueClassConverter = (it.returnType.classifier as? KClass<*>)?.let { kClass -> if (!kClass.isValue) null else { - val constructor = - requireNotNull(kClass.primaryConstructor) { "value class $kClass is expected to have primary constructor, but couldn't obtain it" } + val constructor = requireNotNull(kClass.primaryConstructor) { + "value class $kClass is expected to have primary constructor, but couldn't obtain it" + } val parameter = constructor.parameters.singleOrNull() ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") // there's no need to unwrap if underlying field is nullable @@ -241,18 +254,34 @@ internal fun convertToDataFrame( val kclass = (returnType.classifier as KClass<*>) when { hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable) - kclass == Any::class || preserveClasses.contains(kclass) || preserveProperties.contains(property) || (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || kclass.isValueType -> - DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) - kclass == DataFrame::class && !nullable -> DataColumn.createFrameColumn(it.columnName, values as List) - kclass == DataRow::class -> DataColumn.createColumnGroup(it.columnName, (values as List).concat()) + + kclass == Any::class || + preserveClasses.contains(kclass) || + preserveProperties.contains(property) || + (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || + kclass.isValueType -> + DataColumn.createValueColumn( + name = it.columnName, + values = values, + type = returnType.withNullability(nullable), + ) + + kclass == DataFrame::class && !nullable -> + DataColumn.createFrameColumn( + name = it.columnName, + groups = values as List + ) + + kclass == DataRow::class -> + DataColumn.createColumnGroup( + name = it.columnName, + df = (values as List).concat(), + ) + kclass.isSubclassOf(Iterable::class) -> { val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type if (elementType == null) { - DataColumn.createValueColumn( - it.columnName, - values, - returnType.withNullability(nullable) - ) + DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) } else { val elementClass = (elementType.classifier as? KClass<*>) @@ -264,6 +293,7 @@ internal fun convertToDataFrame( DataColumn.createWithTypeInference(it.columnName, listValues) } + elementClass.isValueType -> { val listType = getListType(elementType).withNullability(nullable) val listValues = values.map { @@ -271,12 +301,22 @@ internal fun convertToDataFrame( } DataColumn.createValueColumn(it.columnName, listValues, listType) } + else -> { val frames = values.map { - if (it == null) DataFrame.empty() - else { + if (it == null) { + DataFrame.empty() + } else { require(it is Iterable<*>) - convertToDataFrame(it, elementClass, emptyList(), excludes, preserveClasses, preserveProperties, maxDepth - 1) + convertToDataFrame( + data = it, + clazz = elementClass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) } } DataColumn.createFrameColumn(it.columnName, frames) @@ -284,13 +324,24 @@ internal fun convertToDataFrame( } } } + else -> { - val df = convertToDataFrame(values, kclass, emptyList(), excludes, preserveClasses, preserveProperties, maxDepth - 1) - DataColumn.createColumnGroup(it.columnName, df) + val df = convertToDataFrame( + data = values, + clazz = kclass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) + DataColumn.createColumnGroup(name = it.columnName, df = df) } } } return if (columns.isEmpty()) { DataFrame.empty(data.count()) - } else dataFrameOf(columns) + } else { + dataFrameOf(columns) + } } From 89ddd20c3fad2b962325fe31f994012dba86ac17 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Mon, 8 Apr 2024 17:08:20 +0200 Subject: [PATCH 04/13] added getterLike checks for KCallable in toDataFrame DSL. reworked property order sorting with multiple constructors. --- .../kotlinx/dataframe/api/toDataFrame.kt | 4 +- .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 20 ++++++- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 52 ++++++++----------- .../kotlinx/dataframe/impl/schema/Utils.kt | 40 +++++++++++++- .../org/jetbrains/kotlinx/dataframe/Utils.kt | 1 + .../kotlinx/dataframe/api/toDataFrame.kt | 4 +- .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 20 ++++++- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 52 ++++++++----------- .../kotlinx/dataframe/impl/schema/Utils.kt | 40 +++++++++++++- .../org/jetbrains/kotlinx/dataframe/Utils.kt | 1 + 10 files changed, 168 insertions(+), 66 deletions(-) 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() } + From a22f863367b9c411640403d9ab40515c9d889be8 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Tue, 9 Apr 2024 11:56:22 +0200 Subject: [PATCH 05/13] new tests taking sortWithConstructors into account --- .../kotlinx/dataframe/api/JavaPojo.java | 8 +++--- .../org/jetbrains/kotlinx/dataframe/Utils.kt | 1 - .../kotlinx/dataframe/api/toDataFrame.kt | 28 +++++++++++++------ .../kotlinx/dataframe/api/JavaPojo.java | 8 +++--- .../org/jetbrains/kotlinx/dataframe/Utils.kt | 1 - .../kotlinx/dataframe/api/toDataFrame.kt | 28 +++++++++++++------ 6 files changed, 48 insertions(+), 26 deletions(-) diff --git a/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java index dddcbd572..34a465ec4 100644 --- a/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java +++ b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -9,7 +9,7 @@ public class JavaPojo { public JavaPojo() {} - public JavaPojo(int a, String b) { + public JavaPojo(String b, int a) { this.a = a; this.b = b; } @@ -51,8 +51,8 @@ public int hashCode() { @Override public String toString() { return "TestPojo{" + - "a=" + a + - ", b='" + b + '\'' + - '}'; + "a=" + a + + ", b='" + b + '\'' + + '}'; } } 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 3d79022a9..c071955e0 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,4 +24,3 @@ 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/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 1eebb0bb7..34480a636 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -193,6 +193,7 @@ class CreateDataFrameTests { fun treatErasedGenericAsAny() { class IncompatibleVersionErrorData(val expected: T, val actual: T) class DeserializedContainerSource(val incompatibility: IncompatibleVersionErrorData<*>) + val functions = listOf(DeserializedContainerSource(IncompatibleVersionErrorData(1, 2))) val df = functions.toDataFrame(maxDepth = 2) @@ -320,11 +321,19 @@ class CreateDataFrameTests { constructor() - constructor(a: Int, b: String) { + constructor(b: String, a: Int) { this.a = a this.b = b } + constructor(b: String) { + this.b = b + } + + constructor(a: Int) { + this.a = a + } + fun getA(): Int = a fun setA(a: Int) { this.a = a @@ -358,18 +367,21 @@ class CreateDataFrameTests { @Test fun `convert POJO to DF`() { - listOf(KotlinPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") - listOf(JavaPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + // even though the names b, a, follow the constructor order + listOf(KotlinPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("b", "a")("bb", 1) + + // cannot read java constructor parameter names with reflection, so sort lexigographically + listOf(JavaPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "bb") - listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("a") + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("bb") - listOf(JavaPojo(1, "a")).toDataFrame { + listOf(JavaPojo("bb", 1)).toDataFrame { properties(JavaPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(JavaPojo(1, "a")).toDataFrame { + listOf(JavaPojo("bb", 1)).toDataFrame { properties(JavaPojo::getB) - } shouldBe dataFrameOf("b")("a") + } shouldBe dataFrameOf("b")("bb") } } diff --git a/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java index dddcbd572..34a465ec4 100644 --- a/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java +++ b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -9,7 +9,7 @@ public class JavaPojo { public JavaPojo() {} - public JavaPojo(int a, String b) { + public JavaPojo(String b, int a) { this.a = a; this.b = b; } @@ -51,8 +51,8 @@ public int hashCode() { @Override public String toString() { return "TestPojo{" + - "a=" + a + - ", b='" + b + '\'' + - '}'; + "a=" + a + + ", b='" + b + '\'' + + '}'; } } 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 3d79022a9..c071955e0 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/Utils.kt @@ -24,4 +24,3 @@ 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/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 1eebb0bb7..34480a636 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -193,6 +193,7 @@ class CreateDataFrameTests { fun treatErasedGenericAsAny() { class IncompatibleVersionErrorData(val expected: T, val actual: T) class DeserializedContainerSource(val incompatibility: IncompatibleVersionErrorData<*>) + val functions = listOf(DeserializedContainerSource(IncompatibleVersionErrorData(1, 2))) val df = functions.toDataFrame(maxDepth = 2) @@ -320,11 +321,19 @@ class CreateDataFrameTests { constructor() - constructor(a: Int, b: String) { + constructor(b: String, a: Int) { this.a = a this.b = b } + constructor(b: String) { + this.b = b + } + + constructor(a: Int) { + this.a = a + } + fun getA(): Int = a fun setA(a: Int) { this.a = a @@ -358,18 +367,21 @@ class CreateDataFrameTests { @Test fun `convert POJO to DF`() { - listOf(KotlinPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") - listOf(JavaPojo(1, "a")).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "a") + // even though the names b, a, follow the constructor order + listOf(KotlinPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("b", "a")("bb", 1) + + // cannot read java constructor parameter names with reflection, so sort lexigographically + listOf(JavaPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "bb") - listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(KotlinPojo(1, "a")).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("a") + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("bb") - listOf(JavaPojo(1, "a")).toDataFrame { + listOf(JavaPojo("bb", 1)).toDataFrame { properties(JavaPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(JavaPojo(1, "a")).toDataFrame { + listOf(JavaPojo("bb", 1)).toDataFrame { properties(JavaPojo::getB) - } shouldBe dataFrameOf("b")("a") + } shouldBe dataFrameOf("b")("bb") } } From f3cc35c2ecb31df06834cade9aba2f3bd3a60496 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 25 Apr 2024 16:32:09 +0200 Subject: [PATCH 06/13] updating isGetterLike and columnName with kdocs and to be more explicit, updating convertToDataFrame implementation to be clearer too. Testing for different types of primitives from java pojo --- .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 49 +++++-- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 137 ++++++++++-------- .../kotlinx/dataframe/api/JavaPojo.java | 43 ++++-- .../kotlinx/dataframe/api/toDataFrame.kt | 24 ++- 4 files changed, 158 insertions(+), 95 deletions(-) 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 852e724ef..7ee866914 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 @@ -340,32 +340,51 @@ internal fun List.joinToCamelCaseString(): String { .replaceFirstChar { it.lowercaseChar() } } +/** @include [KCallable.isGetterLike] */ internal fun KFunction<*>.isGetterLike(): Boolean = - (name.startsWith("get") || name.startsWith("is")) && valueParameters.isEmpty() + (name.startsWith("get") || name.startsWith("is")) && + valueParameters.isEmpty() && + typeParameters.isEmpty() +/** @include [KCallable.isGetterLike] */ +internal fun KProperty<*>.isGetterLike(): Boolean = true + +/** + * Returns `true` if this callable is a getter-like function. + * + * A callable is considered getter-like if it is either a property getter, + * or it's a function with no (type) parameters that starts with "get"/"is". + */ internal fun KCallable<*>.isGetterLike(): Boolean = when (this) { - is KProperty<*> -> true + is KProperty<*> -> isGetterLike() is KFunction<*> -> isGetterLike() else -> false } +/** @include [KCallable.columnName] */ +@PublishedApi +internal val KFunction<*>.columnName: String + get() = findAnnotation()?.name + ?: name + .removePrefix("get") + .removePrefix("is") + .replaceFirstChar { it.lowercase() } + +/** @include [KCallable.columnName] */ @PublishedApi internal val KProperty<*>.columnName: String get() = findAnnotation()?.name ?: name +/** + * Returns the column name for this callable. + * If the callable contains the [ColumnName] annotation, its [ColumnName.name] is returned. + * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. + */ @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 - is KFunction<*> -> - name - .removePrefix("get") - .removePrefix("is") - .replaceFirstChar { it.lowercase() } - - is KProperty<*> -> this.columnName - - else -> name - } + get() = when (this) { + is KFunction<*> -> columnName + is KProperty<*> -> columnName + else -> findAnnotation()?.name ?: 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 183166172..b467859ca 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 @@ -37,7 +37,7 @@ import kotlin.reflect.jvm.isAccessible import kotlin.reflect.jvm.javaField import kotlin.reflect.typeOf -internal val valueTypes = setOf( +private val valueTypes = setOf( String::class, Boolean::class, kotlin.time.Duration::class, @@ -186,21 +186,21 @@ internal fun convertToDataFrame( class ValueClassConverter(val unbox: Method, val box: Method) val valueClassConverter = (it.returnType.classifier as? KClass<*>)?.let { kClass -> - if (!kClass.isValue) null else { - val constructor = requireNotNull(kClass.primaryConstructor) { - "value class $kClass is expected to have primary constructor, but couldn't obtain it" - } - val parameter = constructor.parameters.singleOrNull() - ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") - // there's no need to unwrap if underlying field is nullable - if (parameter.type.isMarkedNullable) return@let null - // box and unbox impl methods are part of binary API of value classes - // https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible - val unbox = kClass.java.getMethod("unbox-impl") - val box = kClass.java.methods.single { it.name == "box-impl" } - val valueClassConverter = ValueClassConverter(unbox, box) - valueClassConverter + if (!kClass.isValue) return@let null + + val constructor = requireNotNull(kClass.primaryConstructor) { + "value class $kClass is expected to have primary constructor, but couldn't obtain it" } + val parameter = constructor.parameters.singleOrNull() + ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") + // there's no need to unwrap if underlying field is nullable + if (parameter.type.isMarkedNullable) return@let null + // box and unbox impl methods are part of binary API of value classes + // https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible + val unbox = kClass.java.getMethod("unbox-impl") + val box = kClass.java.methods.single { it.name == "box-impl" } + val valueClassConverter = ValueClassConverter(unbox, box) + valueClassConverter } (property as? KProperty<*>)?.javaField?.isAccessible = true property.isAccessible = true @@ -245,84 +245,99 @@ internal fun convertToDataFrame( typeOf() } } - val kclass = (returnType.classifier as KClass<*>) + val kClass = returnType.classifier as KClass<*> + + val shouldCreateValueCol = ( + maxDepth <= 0 && + !returnType.shouldBeConvertedToFrameColumn() && + !returnType.shouldBeConvertedToColumnGroup() + ) || + kClass == Any::class || + kClass in preserveClasses || + property in preserveProperties || + kClass.isValueType + + val shouldCreateFrameCol = kClass == DataFrame::class && !nullable + val shouldCreateColumnGroup = kClass == DataRow::class + when { hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable) - kclass == Any::class || - preserveClasses.contains(kclass) || - preserveProperties.contains(property) || - (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || - kclass.isValueType -> + shouldCreateValueCol -> DataColumn.createValueColumn( name = it.columnName, values = values, type = returnType.withNullability(nullable), ) - kclass == DataFrame::class && !nullable -> + shouldCreateFrameCol -> DataColumn.createFrameColumn( name = it.columnName, groups = values as List ) - kclass == DataRow::class -> + shouldCreateColumnGroup -> DataColumn.createColumnGroup( name = it.columnName, df = (values as List).concat(), ) - kclass.isSubclassOf(Iterable::class) -> { - val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type - if (elementType == null) { - DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) - } else { - val elementClass = (elementType.classifier as? KClass<*>) - - when { - elementClass == null -> { - val listValues = values.map { - (it as? Iterable<*>)?.asList() - } + kClass.isSubclassOf(Iterable::class) -> + when (val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type) { + null -> + DataColumn.createValueColumn( + name = it.columnName, + values = values, + type = returnType.withNullability(nullable), + ) + + else -> { + val elementClass = elementType.classifier as? KClass<*> + when { + elementClass == null -> { + val listValues = values.map { + (it as? Iterable<*>)?.asList() + } - DataColumn.createWithTypeInference(it.columnName, listValues) - } + DataColumn.createWithTypeInference(it.columnName, listValues) + } - elementClass.isValueType -> { - val listType = getListType(elementType).withNullability(nullable) - val listValues = values.map { - (it as? Iterable<*>)?.asList() + elementClass.isValueType -> { + val listType = getListType(elementType).withNullability(nullable) + val listValues = values.map { + (it as? Iterable<*>)?.asList() + } + DataColumn.createValueColumn(it.columnName, listValues, listType) } - DataColumn.createValueColumn(it.columnName, listValues, listType) - } - else -> { - val frames = values.map { - if (it == null) { - DataFrame.empty() - } else { - require(it is Iterable<*>) - convertToDataFrame( - data = it, - clazz = elementClass, - roots = emptyList(), - excludes = excludes, - preserveClasses = preserveClasses, - preserveProperties = preserveProperties, - maxDepth = maxDepth - 1, - ) + else -> { + val frames = values.map { + if (it == null) { + DataFrame.empty() + } else { + require(it is Iterable<*>) + convertToDataFrame( + data = it, + clazz = elementClass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) + } } + DataColumn.createFrameColumn(it.columnName, frames) } - DataColumn.createFrameColumn(it.columnName, frames) } } } - } + else -> { val df = convertToDataFrame( data = values, - clazz = kclass, + clazz = kClass, roots = emptyList(), excludes = excludes, preserveClasses = preserveClasses, diff --git a/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java index 34a465ec4..8eccd32fa 100644 --- a/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java +++ b/core/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -6,12 +6,16 @@ public class JavaPojo { private int a; private String b; + private Integer c; + private Number d; public JavaPojo() {} - public JavaPojo(String b, int a) { + public JavaPojo(Number d, Integer c, String b, int a) { this.a = a; this.b = b; + this.c = c; + this.d = d; } public int getA() { @@ -30,29 +34,46 @@ public void setB(String b) { this.b = b; } + public Integer getC() { + return c; + } + + public void setC(Integer c) { + this.c = c; + } + + public Number getD() { + return d; + } + + public void setD(Number d) { + this.d = d; + } + + public static int getNot() { + return 1; + } + @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - JavaPojo testPojo = (JavaPojo) o; - - if (a != testPojo.a) return false; - return Objects.equals(b, testPojo.b); + if (!(o instanceof JavaPojo)) return false; + JavaPojo javaPojo = (JavaPojo) o; + return a == javaPojo.a && Objects.equals(b, javaPojo.b) && Objects.equals(c, javaPojo.c) && Objects.equals(d, javaPojo.d); } @Override public int hashCode() { - int result = a; - result = 31 * result + (b != null ? b.hashCode() : 0); - return result; + return Objects.hash(a, b, c, d); } @Override public String toString() { - return "TestPojo{" + + return "JavaPojo{" + "a=" + a + ", b='" + b + '\'' + + ", c=" + c + + ", d=" + d + '}'; } } diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 34480a636..e691f6273 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -370,17 +370,25 @@ class CreateDataFrameTests { // even though the names b, a, follow the constructor order listOf(KotlinPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("b", "a")("bb", 1) - // cannot read java constructor parameter names with reflection, so sort lexigographically - listOf(JavaPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "bb") - - listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("bb") - - listOf(JavaPojo("bb", 1)).toDataFrame { + // cannot read java constructor parameter names with reflection, so sort lexicographically + listOf(JavaPojo(2.0, null, "bb", 1)).toDataFrame() shouldBe + dataFrameOf( + DataColumn.createValueColumn("a", listOf(1), typeOf()), + DataColumn.createValueColumn("b", listOf("bb"), typeOf()), + DataColumn.createValueColumn("c", listOf(null), typeOf()), + DataColumn.createValueColumn("d", listOf(2.0), typeOf()), + ) + + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe + dataFrameOf("a")(1) + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe + dataFrameOf("b")("bb").groupBy("").concat() + + listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(JavaPojo("bb", 1)).toDataFrame { + listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getB) } shouldBe dataFrameOf("b")("bb") } From 2f79074ee62086ea3095811b492d18e0e87791d8 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 25 Apr 2024 16:33:31 +0200 Subject: [PATCH 07/13] added reflective isArray functions (with tests), so we can treat arrays as values in toDataFrame { properties() } --- .../kotlinx/dataframe/impl/TypeUtils.kt | 103 ++++++++++++++++++ .../kotlinx/dataframe/impl/api/toDataFrame.kt | 4 +- .../kotlinx/dataframe/api/toDataFrame.kt | 17 ++- .../kotlinx/dataframe/types/UtilTests.kt | 52 +++++++++ 4 files changed, 174 insertions(+), 2 deletions(-) diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index c7bee8903..0214a2bce 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -1,3 +1,5 @@ +@file:OptIn(ExperimentalUnsignedTypes::class) + package org.jetbrains.kotlinx.dataframe.impl import org.jetbrains.kotlinx.dataframe.AnyFrame @@ -467,3 +469,104 @@ internal fun nothingType(nullable: Boolean): KType = } else { typeOf>() }.arguments.first().type!! + +@OptIn(ExperimentalUnsignedTypes::class) +private val primitiveArrayClasses = setOf( + BooleanArray::class, + ByteArray::class, + ShortArray::class, + IntArray::class, + LongArray::class, + FloatArray::class, + DoubleArray::class, + CharArray::class, + + UByteArray::class, + UShortArray::class, + UIntArray::class, + ULongArray::class, +) + +/** + * Returns `true` if this class is a primitive array class like `XArray`. + * + * Use [KClass.isArray] to also check for `Array<>`. + */ +internal val KClass<*>.isPrimitiveArray: Boolean + get() = this in primitiveArrayClasses + +/** + * Returns `true` if this class is an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [KClass.isPrimitiveArray] to only check for primitive arrays. + */ +internal val KClass<*>.isArray: Boolean + get() = this in primitiveArrayClasses || + this.qualifiedName == Array::class.qualifiedName // instance check fails + +/** + * Returns `true` if this type is of a primitive array like `XArray`. + * + * Use [KType.isArray] to also check for `Array<>`. + */ +internal val KType.isPrimitiveArray: Boolean + get() = + if (arguments.isNotEmpty()) { + // Catching https://github.com/Kotlin/dataframe/issues/678 + // as typeOf>().classifier == IntArray::class + false + } else { + (classifier as? KClass<*>)?.isPrimitiveArray == true + } + + +/** + * Returns `true` if this type is of an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [KType.isPrimitiveArray] to only check for primitive arrays. + */ +internal val KType.isArray: Boolean + get() = (classifier as? KClass<*>)?.isArray == true + +/** + * Returns `true` if this object is a primitive array like `XArray`. + * + * Use [Any.isArray] to also check for the `Array<>` object. + */ +internal val Any.isPrimitiveArray: Boolean + get() = this::class.isPrimitiveArray + +/** + * Returns `true` if this object is an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [Any.isPrimitiveArray] to only check for primitive arrays. + */ +internal val Any.isArray: Boolean + get() = this::class.isArray + +/** + * Returns this array object as a list of values. + * + * @throws IllegalArgumentException if this object is not an array + */ +internal fun Any.asArrayToList(): List<*> { + require(this.isArray) { "Not an array" } + return when (this) { + is BooleanArray -> toList() + is ByteArray -> toList() + is ShortArray -> toList() + is IntArray -> toList() + is LongArray -> toList() + is FloatArray -> toList() + is DoubleArray -> toList() + is CharArray -> toList() + + is UByteArray -> toList() + is UShortArray -> toList() + is UIntArray -> toList() + is ULongArray -> toList() + + is Array<*> -> toList() + else -> error("Not an array") + } +} 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 b467859ca..8586c7300 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,6 +18,7 @@ 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.isArray import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors @@ -51,7 +52,8 @@ internal val KClass<*>.isValueType: Boolean this in valueTypes || this.isSubclassOf(Number::class) || this.isSubclassOf(Enum::class) || - this.isSubclassOf(Temporal::class) + this.isSubclassOf(Temporal::class) || + this.isArray internal class CreateDataFrameDslImpl( override val source: Iterable, diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index e691f6273..fba1ffdb7 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -1,9 +1,9 @@ package org.jetbrains.kotlinx.dataframe.api 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.alsoDebug import org.jetbrains.kotlinx.dataframe.annotations.DataSchema import org.jetbrains.kotlinx.dataframe.columns.ColumnKind import org.jetbrains.kotlinx.dataframe.kind @@ -392,4 +392,19 @@ class CreateDataFrameTests { properties(JavaPojo::getB) } shouldBe dataFrameOf("b")("bb") } + + data class Arrays(val a: IntArray, val b: Array, val c: Array) + + @Test + fun `arrays in to DF`() { + val df = listOf( + Arrays(intArrayOf(1, 2), arrayOf(3, 4), arrayOf(5, null)) + ).toDataFrame(maxDepth = Int.MAX_VALUE) + + df.schema() shouldBe dataFrameOf( + DataColumn.createValueColumn("a", listOf(intArrayOf(1, 2)), typeOf()), + DataColumn.createValueColumn("b", listOf(arrayOf(3, 4)), typeOf>()), + DataColumn.createValueColumn("c", listOf(arrayOf(5, null)), typeOf>()), + ).schema() + } } diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt index 28c552af9..e909d9fec 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt @@ -1,12 +1,15 @@ package org.jetbrains.kotlinx.dataframe.types import io.kotest.matchers.shouldBe +import org.jetbrains.kotlinx.dataframe.impl.asArrayToList import org.jetbrains.kotlinx.dataframe.impl.commonParent import org.jetbrains.kotlinx.dataframe.impl.commonParents import org.jetbrains.kotlinx.dataframe.impl.commonType import org.jetbrains.kotlinx.dataframe.impl.commonTypeListifyValues import org.jetbrains.kotlinx.dataframe.impl.createType import org.jetbrains.kotlinx.dataframe.impl.guessValueType +import org.jetbrains.kotlinx.dataframe.impl.isArray +import org.jetbrains.kotlinx.dataframe.impl.isPrimitiveArray import org.jetbrains.kotlinx.dataframe.impl.nothingType import org.jetbrains.kotlinx.dataframe.impl.replaceGenericTypeParametersWithUpperbound import org.junit.Test @@ -17,6 +20,55 @@ import kotlin.reflect.typeOf class UtilTests { + @OptIn(ExperimentalUnsignedTypes::class) + @Test + fun `isArray tests`() { + // KClass isArray + BooleanArray::class.isArray shouldBe true + UIntArray::class.isArray shouldBe true + Array::class.isArray shouldBe true + + // KClass isPrimitiveArray + BooleanArray::class.isPrimitiveArray shouldBe true + UIntArray::class.isPrimitiveArray shouldBe true + Array::class.isPrimitiveArray shouldBe false + + // KType isArray + typeOf().isArray shouldBe true + typeOf().isArray shouldBe true + typeOf>().isArray shouldBe true + typeOf>().isArray shouldBe true + typeOf>().isArray shouldBe true + + // KType isPrimitiveArray + typeOf().isPrimitiveArray shouldBe true + typeOf().isPrimitiveArray shouldBe true + typeOf>().isPrimitiveArray shouldBe false + typeOf>().isPrimitiveArray shouldBe false + typeOf>().isPrimitiveArray shouldBe false + + // Any isArray + booleanArrayOf().isArray shouldBe true + uintArrayOf().isArray shouldBe true + arrayOf(1).isArray shouldBe true + arrayOf(1, null).isArray shouldBe true + arrayOfNulls(1).isArray shouldBe true + + // Any isPrimitiveArray + booleanArrayOf().isPrimitiveArray shouldBe true + uintArrayOf().isPrimitiveArray shouldBe true + arrayOf(1).isPrimitiveArray shouldBe false + arrayOf(1, null).isPrimitiveArray shouldBe false + arrayOfNulls(1).isPrimitiveArray shouldBe false + + // Any asArrayToList + booleanArrayOf(true, false).asArrayToList() shouldBe listOf(true, false) + uintArrayOf(1u, 2u).asArrayToList() shouldBe listOf(1u, 2u) + arrayOf(1, 2).asArrayToList() shouldBe listOf(1, 2) + arrayOf(1, null).asArrayToList() shouldBe listOf(1, null) + arrayOfNulls(1).asArrayToList() shouldBe listOf(null) + } + @Test fun commonParentsTests() { commonParents(Int::class, Int::class) shouldBe listOf(Int::class) From ffd4dce50b077069622ade48b73e2ac685fd0b84 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 25 Apr 2024 16:34:34 +0200 Subject: [PATCH 08/13] Fixed rendering of Array types and added value rendering for arrays as well :) --- .../kotlinx/dataframe/impl/Rendering.kt | 8 + .../kotlinx/dataframe/impl/TypeUtils.kt | 103 +++++++++++++ .../jetbrains/kotlinx/dataframe/impl/Utils.kt | 59 ++++++-- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 141 ++++++++++-------- .../jetbrains/kotlinx/dataframe/io/string.kt | 21 ++- .../kotlinx/dataframe/api/JavaPojo.java | 43 ++++-- .../kotlinx/dataframe/api/toDataFrame.kt | 41 +++-- .../dataframe/rendering/RenderingTests.kt | 19 +++ .../kotlinx/dataframe/types/UtilTests.kt | 52 +++++++ .../kotlinx/dataframe/impl/Rendering.kt | 8 + .../jetbrains/kotlinx/dataframe/io/string.kt | 21 ++- .../dataframe/rendering/RenderingTests.kt | 19 +++ 12 files changed, 426 insertions(+), 109 deletions(-) diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt index ef13ebeea..1326285b8 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt @@ -16,7 +16,9 @@ import java.net.URL import java.time.LocalDateTime import java.time.LocalTime import kotlin.reflect.KType +import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.jvm.jvmErasure +import kotlin.reflect.typeOf internal fun String.truncate(limit: Int): RenderedContent = if (limit in 1 until length) { if (limit < 4) RenderedContent.truncatedText("...", this) @@ -57,6 +59,12 @@ internal fun renderType(type: KType?): String { else -> { val fullName = type.jvmErasure.qualifiedName ?: return type.toString() val name = when { + + // catching cases like `typeOf>().jvmErasure.qualifiedName == "IntArray"` + // https://github.com/Kotlin/dataframe/issues/678 + type.isSubtypeOf(typeOf>()) -> + "Array" + type.classifier == URL::class -> fullName.removePrefix("java.net.") diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index c7bee8903..0214a2bce 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -1,3 +1,5 @@ +@file:OptIn(ExperimentalUnsignedTypes::class) + package org.jetbrains.kotlinx.dataframe.impl import org.jetbrains.kotlinx.dataframe.AnyFrame @@ -467,3 +469,104 @@ internal fun nothingType(nullable: Boolean): KType = } else { typeOf>() }.arguments.first().type!! + +@OptIn(ExperimentalUnsignedTypes::class) +private val primitiveArrayClasses = setOf( + BooleanArray::class, + ByteArray::class, + ShortArray::class, + IntArray::class, + LongArray::class, + FloatArray::class, + DoubleArray::class, + CharArray::class, + + UByteArray::class, + UShortArray::class, + UIntArray::class, + ULongArray::class, +) + +/** + * Returns `true` if this class is a primitive array class like `XArray`. + * + * Use [KClass.isArray] to also check for `Array<>`. + */ +internal val KClass<*>.isPrimitiveArray: Boolean + get() = this in primitiveArrayClasses + +/** + * Returns `true` if this class is an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [KClass.isPrimitiveArray] to only check for primitive arrays. + */ +internal val KClass<*>.isArray: Boolean + get() = this in primitiveArrayClasses || + this.qualifiedName == Array::class.qualifiedName // instance check fails + +/** + * Returns `true` if this type is of a primitive array like `XArray`. + * + * Use [KType.isArray] to also check for `Array<>`. + */ +internal val KType.isPrimitiveArray: Boolean + get() = + if (arguments.isNotEmpty()) { + // Catching https://github.com/Kotlin/dataframe/issues/678 + // as typeOf>().classifier == IntArray::class + false + } else { + (classifier as? KClass<*>)?.isPrimitiveArray == true + } + + +/** + * Returns `true` if this type is of an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [KType.isPrimitiveArray] to only check for primitive arrays. + */ +internal val KType.isArray: Boolean + get() = (classifier as? KClass<*>)?.isArray == true + +/** + * Returns `true` if this object is a primitive array like `XArray`. + * + * Use [Any.isArray] to also check for the `Array<>` object. + */ +internal val Any.isPrimitiveArray: Boolean + get() = this::class.isPrimitiveArray + +/** + * Returns `true` if this object is an array, either a primitive array like `XArray` or `Array<>`. + * + * Use [Any.isPrimitiveArray] to only check for primitive arrays. + */ +internal val Any.isArray: Boolean + get() = this::class.isArray + +/** + * Returns this array object as a list of values. + * + * @throws IllegalArgumentException if this object is not an array + */ +internal fun Any.asArrayToList(): List<*> { + require(this.isArray) { "Not an array" } + return when (this) { + is BooleanArray -> toList() + is ByteArray -> toList() + is ShortArray -> toList() + is IntArray -> toList() + is LongArray -> toList() + is FloatArray -> toList() + is DoubleArray -> toList() + is CharArray -> toList() + + is UByteArray -> toList() + is UShortArray -> toList() + is UIntArray -> toList() + is ULongArray -> toList() + + is Array<*> -> toList() + else -> error("Not an array") + } +} 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 852e724ef..5f1241a21 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 @@ -340,32 +340,61 @@ internal fun List.joinToCamelCaseString(): String { .replaceFirstChar { it.lowercaseChar() } } +/** Returns `true` if this callable is a getter-like function. + * + * A callable is considered getter-like if it is either a property getter, + * or it's a function with no (type) parameters that starts with "get"/"is". */ internal fun KFunction<*>.isGetterLike(): Boolean = - (name.startsWith("get") || name.startsWith("is")) && valueParameters.isEmpty() + (name.startsWith("get") || name.startsWith("is")) && + valueParameters.isEmpty() && + typeParameters.isEmpty() + +/** Returns `true` if this callable is a getter-like function. + * + * A callable is considered getter-like if it is either a property getter, + * or it's a function with no (type) parameters that starts with "get"/"is". */ +internal fun KProperty<*>.isGetterLike(): Boolean = true +/** + * Returns `true` if this callable is a getter-like function. + * + * A callable is considered getter-like if it is either a property getter, + * or it's a function with no (type) parameters that starts with "get"/"is". + */ internal fun KCallable<*>.isGetterLike(): Boolean = when (this) { - is KProperty<*> -> true + is KProperty<*> -> isGetterLike() is KFunction<*> -> isGetterLike() else -> false } +/** Returns the column name for this callable. + * If the callable contains the [ColumnName][org.jetbrains.kotlinx.dataframe.annotations.ColumnName] annotation, its [ColumnName.name][org.jetbrains.kotlinx.dataframe.annotations.ColumnName.name] is returned. + * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. */ +@PublishedApi +internal val KFunction<*>.columnName: String + get() = findAnnotation()?.name + ?: name + .removePrefix("get") + .removePrefix("is") + .replaceFirstChar { it.lowercase() } + +/** Returns the column name for this callable. + * If the callable contains the [ColumnName][org.jetbrains.kotlinx.dataframe.annotations.ColumnName] annotation, its [ColumnName.name][org.jetbrains.kotlinx.dataframe.annotations.ColumnName.name] is returned. + * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. */ @PublishedApi internal val KProperty<*>.columnName: String get() = findAnnotation()?.name ?: name +/** + * Returns the column name for this callable. + * If the callable contains the [ColumnName] annotation, its [ColumnName.name] is returned. + * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. + */ @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 - is KFunction<*> -> - name - .removePrefix("get") - .removePrefix("is") - .replaceFirstChar { it.lowercase() } - - is KProperty<*> -> this.columnName - - else -> name - } + get() = when (this) { + is KFunction<*> -> columnName + is KProperty<*> -> columnName + else -> findAnnotation()?.name ?: 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 183166172..8586c7300 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,6 +18,7 @@ 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.isArray import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors @@ -37,7 +38,7 @@ import kotlin.reflect.jvm.isAccessible import kotlin.reflect.jvm.javaField import kotlin.reflect.typeOf -internal val valueTypes = setOf( +private val valueTypes = setOf( String::class, Boolean::class, kotlin.time.Duration::class, @@ -51,7 +52,8 @@ internal val KClass<*>.isValueType: Boolean this in valueTypes || this.isSubclassOf(Number::class) || this.isSubclassOf(Enum::class) || - this.isSubclassOf(Temporal::class) + this.isSubclassOf(Temporal::class) || + this.isArray internal class CreateDataFrameDslImpl( override val source: Iterable, @@ -186,21 +188,21 @@ internal fun convertToDataFrame( class ValueClassConverter(val unbox: Method, val box: Method) val valueClassConverter = (it.returnType.classifier as? KClass<*>)?.let { kClass -> - if (!kClass.isValue) null else { - val constructor = requireNotNull(kClass.primaryConstructor) { - "value class $kClass is expected to have primary constructor, but couldn't obtain it" - } - val parameter = constructor.parameters.singleOrNull() - ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") - // there's no need to unwrap if underlying field is nullable - if (parameter.type.isMarkedNullable) return@let null - // box and unbox impl methods are part of binary API of value classes - // https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible - val unbox = kClass.java.getMethod("unbox-impl") - val box = kClass.java.methods.single { it.name == "box-impl" } - val valueClassConverter = ValueClassConverter(unbox, box) - valueClassConverter + if (!kClass.isValue) return@let null + + val constructor = requireNotNull(kClass.primaryConstructor) { + "value class $kClass is expected to have primary constructor, but couldn't obtain it" } + val parameter = constructor.parameters.singleOrNull() + ?: error("conversion of value class $kClass with multiple parameters in constructor is not yet supported") + // there's no need to unwrap if underlying field is nullable + if (parameter.type.isMarkedNullable) return@let null + // box and unbox impl methods are part of binary API of value classes + // https://youtrack.jetbrains.com/issue/KT-50518/Boxing-Unboxing-methods-for-JvmInline-value-classes-should-be-public-accessible + val unbox = kClass.java.getMethod("unbox-impl") + val box = kClass.java.methods.single { it.name == "box-impl" } + val valueClassConverter = ValueClassConverter(unbox, box) + valueClassConverter } (property as? KProperty<*>)?.javaField?.isAccessible = true property.isAccessible = true @@ -245,84 +247,99 @@ internal fun convertToDataFrame( typeOf() } } - val kclass = (returnType.classifier as KClass<*>) + val kClass = returnType.classifier as KClass<*> + + val shouldCreateValueCol = ( + maxDepth <= 0 && + !returnType.shouldBeConvertedToFrameColumn() && + !returnType.shouldBeConvertedToColumnGroup() + ) || + kClass == Any::class || + kClass in preserveClasses || + property in preserveProperties || + kClass.isValueType + + val shouldCreateFrameCol = kClass == DataFrame::class && !nullable + val shouldCreateColumnGroup = kClass == DataRow::class + when { hasExceptions -> DataColumn.createWithTypeInference(it.columnName, values, nullable) - kclass == Any::class || - preserveClasses.contains(kclass) || - preserveProperties.contains(property) || - (maxDepth <= 0 && !returnType.shouldBeConvertedToFrameColumn() && !returnType.shouldBeConvertedToColumnGroup()) || - kclass.isValueType -> + shouldCreateValueCol -> DataColumn.createValueColumn( name = it.columnName, values = values, type = returnType.withNullability(nullable), ) - kclass == DataFrame::class && !nullable -> + shouldCreateFrameCol -> DataColumn.createFrameColumn( name = it.columnName, groups = values as List ) - kclass == DataRow::class -> + shouldCreateColumnGroup -> DataColumn.createColumnGroup( name = it.columnName, df = (values as List).concat(), ) - kclass.isSubclassOf(Iterable::class) -> { - val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type - if (elementType == null) { - DataColumn.createValueColumn(it.columnName, values, returnType.withNullability(nullable)) - } else { - val elementClass = (elementType.classifier as? KClass<*>) - - when { - elementClass == null -> { - val listValues = values.map { - (it as? Iterable<*>)?.asList() - } + kClass.isSubclassOf(Iterable::class) -> + when (val elementType = returnType.projectUpTo(Iterable::class).arguments.firstOrNull()?.type) { + null -> + DataColumn.createValueColumn( + name = it.columnName, + values = values, + type = returnType.withNullability(nullable), + ) + + else -> { + val elementClass = elementType.classifier as? KClass<*> + when { + elementClass == null -> { + val listValues = values.map { + (it as? Iterable<*>)?.asList() + } - DataColumn.createWithTypeInference(it.columnName, listValues) - } + DataColumn.createWithTypeInference(it.columnName, listValues) + } - elementClass.isValueType -> { - val listType = getListType(elementType).withNullability(nullable) - val listValues = values.map { - (it as? Iterable<*>)?.asList() + elementClass.isValueType -> { + val listType = getListType(elementType).withNullability(nullable) + val listValues = values.map { + (it as? Iterable<*>)?.asList() + } + DataColumn.createValueColumn(it.columnName, listValues, listType) } - DataColumn.createValueColumn(it.columnName, listValues, listType) - } - else -> { - val frames = values.map { - if (it == null) { - DataFrame.empty() - } else { - require(it is Iterable<*>) - convertToDataFrame( - data = it, - clazz = elementClass, - roots = emptyList(), - excludes = excludes, - preserveClasses = preserveClasses, - preserveProperties = preserveProperties, - maxDepth = maxDepth - 1, - ) + else -> { + val frames = values.map { + if (it == null) { + DataFrame.empty() + } else { + require(it is Iterable<*>) + convertToDataFrame( + data = it, + clazz = elementClass, + roots = emptyList(), + excludes = excludes, + preserveClasses = preserveClasses, + preserveProperties = preserveProperties, + maxDepth = maxDepth - 1, + ) + } } + DataColumn.createFrameColumn(it.columnName, frames) } - DataColumn.createFrameColumn(it.columnName, frames) } } } - } + else -> { val df = convertToDataFrame( data = values, - clazz = kclass, + clazz = kClass, roots = emptyList(), excludes = excludes, preserveClasses = preserveClasses, diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt index 7aacb998c..a69c61322 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt @@ -7,6 +7,8 @@ import org.jetbrains.kotlinx.dataframe.api.columnsCount import org.jetbrains.kotlinx.dataframe.api.isNumber import org.jetbrains.kotlinx.dataframe.api.take import org.jetbrains.kotlinx.dataframe.api.toColumn +import org.jetbrains.kotlinx.dataframe.impl.asArrayToList +import org.jetbrains.kotlinx.dataframe.impl.isArray import org.jetbrains.kotlinx.dataframe.impl.owner import org.jetbrains.kotlinx.dataframe.impl.renderType import org.jetbrains.kotlinx.dataframe.impl.scale @@ -124,15 +126,16 @@ internal fun AnyRow.getVisibleValues(): List> { internal fun AnyRow.renderToString(): String { val values = getVisibleValues() if (values.isEmpty()) return "{ }" - return values - .map { "${it.first}:${renderValueForStdout(it.second).truncatedContent}" } - .joinToString(prefix = "{ ", postfix = " }") + return values.joinToString( + prefix = "{ ", + postfix = " }" + ) { "${it.first}:${renderValueForStdout(it.second).truncatedContent}" } } internal fun AnyRow.renderToStringTable(forHtml: Boolean = false): String { if (columnsCount() == 0) return "" val pairs = owner.columns().map { it.name() to renderValueForRowTable(it[index], forHtml) } - val width = pairs.map { it.first.length + it.second.textLength }.maxOrNull()!! + 4 + val width = pairs.maxOf { it.first.length + it.second.textLength } + 4 return pairs.joinToString("\n") { it.first + " ".repeat(width - it.first.length - it.second.textLength) + it.second.truncatedContent } @@ -165,14 +168,20 @@ internal fun renderValueForStdout( renderValueToString(value, decimalFormat).truncate(limit) .let { it.copy(truncatedContent = it.truncatedContent.escapeNewLines()) } -internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalFormat) = +internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalFormat): String = when (value) { is AnyFrame -> "[${value.size}]".let { if (value.nrow == 1) it + " " + value[0].toString() else it } is Double -> value.format(decimalFormat) is Float -> value.format(decimalFormat) is BigDecimal -> value.format(decimalFormat) is List<*> -> if (value.isEmpty()) "[ ]" else value.toString() - else -> value.toString() + is Array<*> -> if (value.isEmpty()) "[ ]" else value.toList().toString() + else -> + if (value?.isArray == true) { + renderValueToString(value.asArrayToList(), decimalFormat) + } else { + value.toString() + } } internal fun internallyRenderable(value: Any?): Boolean { diff --git a/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java index 34a465ec4..8eccd32fa 100644 --- a/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java +++ b/core/generated-sources/src/test/java/org/jetbrains/kotlinx/dataframe/api/JavaPojo.java @@ -6,12 +6,16 @@ public class JavaPojo { private int a; private String b; + private Integer c; + private Number d; public JavaPojo() {} - public JavaPojo(String b, int a) { + public JavaPojo(Number d, Integer c, String b, int a) { this.a = a; this.b = b; + this.c = c; + this.d = d; } public int getA() { @@ -30,29 +34,46 @@ public void setB(String b) { this.b = b; } + public Integer getC() { + return c; + } + + public void setC(Integer c) { + this.c = c; + } + + public Number getD() { + return d; + } + + public void setD(Number d) { + this.d = d; + } + + public static int getNot() { + return 1; + } + @Override public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - JavaPojo testPojo = (JavaPojo) o; - - if (a != testPojo.a) return false; - return Objects.equals(b, testPojo.b); + if (!(o instanceof JavaPojo)) return false; + JavaPojo javaPojo = (JavaPojo) o; + return a == javaPojo.a && Objects.equals(b, javaPojo.b) && Objects.equals(c, javaPojo.c) && Objects.equals(d, javaPojo.d); } @Override public int hashCode() { - int result = a; - result = 31 * result + (b != null ? b.hashCode() : 0); - return result; + return Objects.hash(a, b, c, d); } @Override public String toString() { - return "TestPojo{" + + return "JavaPojo{" + "a=" + a + ", b='" + b + '\'' + + ", c=" + c + + ", d=" + d + '}'; } } diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 34480a636..fba1ffdb7 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -1,9 +1,9 @@ package org.jetbrains.kotlinx.dataframe.api 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.alsoDebug import org.jetbrains.kotlinx.dataframe.annotations.DataSchema import org.jetbrains.kotlinx.dataframe.columns.ColumnKind import org.jetbrains.kotlinx.dataframe.kind @@ -370,18 +370,41 @@ class CreateDataFrameTests { // even though the names b, a, follow the constructor order listOf(KotlinPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("b", "a")("bb", 1) - // cannot read java constructor parameter names with reflection, so sort lexigographically - listOf(JavaPojo("bb", 1)).toDataFrame() shouldBe dataFrameOf("a", "b")(1, "bb") - - listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe dataFrameOf("b")("bb") - - listOf(JavaPojo("bb", 1)).toDataFrame { + // cannot read java constructor parameter names with reflection, so sort lexicographically + listOf(JavaPojo(2.0, null, "bb", 1)).toDataFrame() shouldBe + dataFrameOf( + DataColumn.createValueColumn("a", listOf(1), typeOf()), + DataColumn.createValueColumn("b", listOf("bb"), typeOf()), + DataColumn.createValueColumn("c", listOf(null), typeOf()), + DataColumn.createValueColumn("d", listOf(2.0), typeOf()), + ) + + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe + dataFrameOf("a")(1) + listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe + dataFrameOf("b")("bb").groupBy("").concat() + + listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getA) } shouldBe dataFrameOf("a")(1) - listOf(JavaPojo("bb", 1)).toDataFrame { + listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getB) } shouldBe dataFrameOf("b")("bb") } + + data class Arrays(val a: IntArray, val b: Array, val c: Array) + + @Test + fun `arrays in to DF`() { + val df = listOf( + Arrays(intArrayOf(1, 2), arrayOf(3, 4), arrayOf(5, null)) + ).toDataFrame(maxDepth = Int.MAX_VALUE) + + df.schema() shouldBe dataFrameOf( + DataColumn.createValueColumn("a", listOf(intArrayOf(1, 2)), typeOf()), + DataColumn.createValueColumn("b", listOf(arrayOf(3, 4)), typeOf>()), + DataColumn.createValueColumn("c", listOf(arrayOf(5, null)), typeOf>()), + ).schema() + } } diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt index 211272f5d..27d9b6b15 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt @@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.dataframe.rendering import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.string.shouldNotContain +import org.jetbrains.kotlinx.dataframe.DataColumn import org.jetbrains.kotlinx.dataframe.api.add import org.jetbrains.kotlinx.dataframe.api.asColumnGroup import org.jetbrains.kotlinx.dataframe.api.columnOf @@ -11,7 +12,9 @@ import org.jetbrains.kotlinx.dataframe.api.emptyDataFrame import org.jetbrains.kotlinx.dataframe.api.group import org.jetbrains.kotlinx.dataframe.api.into import org.jetbrains.kotlinx.dataframe.api.move +import org.jetbrains.kotlinx.dataframe.api.named import org.jetbrains.kotlinx.dataframe.api.parse +import org.jetbrains.kotlinx.dataframe.api.schema import org.jetbrains.kotlinx.dataframe.api.toDataFrame import org.jetbrains.kotlinx.dataframe.io.DisplayConfiguration import org.jetbrains.kotlinx.dataframe.io.escapeHTML @@ -174,4 +177,20 @@ class RenderingTests : TestBase() { dfGroup.name.lastName.maxWidth() shouldBe 1 dfGroup.name.firstName.secondName.maxWidth() shouldBe 1 } + + @Test + fun `render array types correctly`() { + val df = dataFrameOf( + columnOf(1, null).named("a"), + columnOf(intArrayOf(1), intArrayOf(2)).named("b"), + // TODO https://github.com/Kotlin/dataframe/issues/679 + // columnOf(arrayOf(1), arrayOf(2)).named("d"), + DataColumn.createValueColumn("c", listOf(arrayOf(1), arrayOf(2))), + columnOf(arrayOf(1, null), arrayOf(2, null)).named("d"), + ) + + val schema = df.schema() + val rendered = schema.toString() + rendered shouldBe "a: Int?\nb: IntArray\nc: Array\nd: Array" + } } diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt index 28c552af9..e909d9fec 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt @@ -1,12 +1,15 @@ package org.jetbrains.kotlinx.dataframe.types import io.kotest.matchers.shouldBe +import org.jetbrains.kotlinx.dataframe.impl.asArrayToList import org.jetbrains.kotlinx.dataframe.impl.commonParent import org.jetbrains.kotlinx.dataframe.impl.commonParents import org.jetbrains.kotlinx.dataframe.impl.commonType import org.jetbrains.kotlinx.dataframe.impl.commonTypeListifyValues import org.jetbrains.kotlinx.dataframe.impl.createType import org.jetbrains.kotlinx.dataframe.impl.guessValueType +import org.jetbrains.kotlinx.dataframe.impl.isArray +import org.jetbrains.kotlinx.dataframe.impl.isPrimitiveArray import org.jetbrains.kotlinx.dataframe.impl.nothingType import org.jetbrains.kotlinx.dataframe.impl.replaceGenericTypeParametersWithUpperbound import org.junit.Test @@ -17,6 +20,55 @@ import kotlin.reflect.typeOf class UtilTests { + @OptIn(ExperimentalUnsignedTypes::class) + @Test + fun `isArray tests`() { + // KClass isArray + BooleanArray::class.isArray shouldBe true + UIntArray::class.isArray shouldBe true + Array::class.isArray shouldBe true + + // KClass isPrimitiveArray + BooleanArray::class.isPrimitiveArray shouldBe true + UIntArray::class.isPrimitiveArray shouldBe true + Array::class.isPrimitiveArray shouldBe false + + // KType isArray + typeOf().isArray shouldBe true + typeOf().isArray shouldBe true + typeOf>().isArray shouldBe true + typeOf>().isArray shouldBe true + typeOf>().isArray shouldBe true + + // KType isPrimitiveArray + typeOf().isPrimitiveArray shouldBe true + typeOf().isPrimitiveArray shouldBe true + typeOf>().isPrimitiveArray shouldBe false + typeOf>().isPrimitiveArray shouldBe false + typeOf>().isPrimitiveArray shouldBe false + + // Any isArray + booleanArrayOf().isArray shouldBe true + uintArrayOf().isArray shouldBe true + arrayOf(1).isArray shouldBe true + arrayOf(1, null).isArray shouldBe true + arrayOfNulls(1).isArray shouldBe true + + // Any isPrimitiveArray + booleanArrayOf().isPrimitiveArray shouldBe true + uintArrayOf().isPrimitiveArray shouldBe true + arrayOf(1).isPrimitiveArray shouldBe false + arrayOf(1, null).isPrimitiveArray shouldBe false + arrayOfNulls(1).isPrimitiveArray shouldBe false + + // Any asArrayToList + booleanArrayOf(true, false).asArrayToList() shouldBe listOf(true, false) + uintArrayOf(1u, 2u).asArrayToList() shouldBe listOf(1u, 2u) + arrayOf(1, 2).asArrayToList() shouldBe listOf(1, 2) + arrayOf(1, null).asArrayToList() shouldBe listOf(1, null) + arrayOfNulls(1).asArrayToList() shouldBe listOf(null) + } + @Test fun commonParentsTests() { commonParents(Int::class, Int::class) shouldBe listOf(Int::class) diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt index ef13ebeea..1326285b8 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt @@ -16,7 +16,9 @@ import java.net.URL import java.time.LocalDateTime import java.time.LocalTime import kotlin.reflect.KType +import kotlin.reflect.full.isSubtypeOf import kotlin.reflect.jvm.jvmErasure +import kotlin.reflect.typeOf internal fun String.truncate(limit: Int): RenderedContent = if (limit in 1 until length) { if (limit < 4) RenderedContent.truncatedText("...", this) @@ -57,6 +59,12 @@ internal fun renderType(type: KType?): String { else -> { val fullName = type.jvmErasure.qualifiedName ?: return type.toString() val name = when { + + // catching cases like `typeOf>().jvmErasure.qualifiedName == "IntArray"` + // https://github.com/Kotlin/dataframe/issues/678 + type.isSubtypeOf(typeOf>()) -> + "Array" + type.classifier == URL::class -> fullName.removePrefix("java.net.") diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt index 7aacb998c..a69c61322 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt @@ -7,6 +7,8 @@ import org.jetbrains.kotlinx.dataframe.api.columnsCount import org.jetbrains.kotlinx.dataframe.api.isNumber import org.jetbrains.kotlinx.dataframe.api.take import org.jetbrains.kotlinx.dataframe.api.toColumn +import org.jetbrains.kotlinx.dataframe.impl.asArrayToList +import org.jetbrains.kotlinx.dataframe.impl.isArray import org.jetbrains.kotlinx.dataframe.impl.owner import org.jetbrains.kotlinx.dataframe.impl.renderType import org.jetbrains.kotlinx.dataframe.impl.scale @@ -124,15 +126,16 @@ internal fun AnyRow.getVisibleValues(): List> { internal fun AnyRow.renderToString(): String { val values = getVisibleValues() if (values.isEmpty()) return "{ }" - return values - .map { "${it.first}:${renderValueForStdout(it.second).truncatedContent}" } - .joinToString(prefix = "{ ", postfix = " }") + return values.joinToString( + prefix = "{ ", + postfix = " }" + ) { "${it.first}:${renderValueForStdout(it.second).truncatedContent}" } } internal fun AnyRow.renderToStringTable(forHtml: Boolean = false): String { if (columnsCount() == 0) return "" val pairs = owner.columns().map { it.name() to renderValueForRowTable(it[index], forHtml) } - val width = pairs.map { it.first.length + it.second.textLength }.maxOrNull()!! + 4 + val width = pairs.maxOf { it.first.length + it.second.textLength } + 4 return pairs.joinToString("\n") { it.first + " ".repeat(width - it.first.length - it.second.textLength) + it.second.truncatedContent } @@ -165,14 +168,20 @@ internal fun renderValueForStdout( renderValueToString(value, decimalFormat).truncate(limit) .let { it.copy(truncatedContent = it.truncatedContent.escapeNewLines()) } -internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalFormat) = +internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalFormat): String = when (value) { is AnyFrame -> "[${value.size}]".let { if (value.nrow == 1) it + " " + value[0].toString() else it } is Double -> value.format(decimalFormat) is Float -> value.format(decimalFormat) is BigDecimal -> value.format(decimalFormat) is List<*> -> if (value.isEmpty()) "[ ]" else value.toString() - else -> value.toString() + is Array<*> -> if (value.isEmpty()) "[ ]" else value.toList().toString() + else -> + if (value?.isArray == true) { + renderValueToString(value.asArrayToList(), decimalFormat) + } else { + value.toString() + } } internal fun internallyRenderable(value: Any?): Boolean { diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt index 211272f5d..27d9b6b15 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/rendering/RenderingTests.kt @@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.dataframe.rendering import io.kotest.matchers.shouldBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.string.shouldNotContain +import org.jetbrains.kotlinx.dataframe.DataColumn import org.jetbrains.kotlinx.dataframe.api.add import org.jetbrains.kotlinx.dataframe.api.asColumnGroup import org.jetbrains.kotlinx.dataframe.api.columnOf @@ -11,7 +12,9 @@ import org.jetbrains.kotlinx.dataframe.api.emptyDataFrame import org.jetbrains.kotlinx.dataframe.api.group import org.jetbrains.kotlinx.dataframe.api.into import org.jetbrains.kotlinx.dataframe.api.move +import org.jetbrains.kotlinx.dataframe.api.named import org.jetbrains.kotlinx.dataframe.api.parse +import org.jetbrains.kotlinx.dataframe.api.schema import org.jetbrains.kotlinx.dataframe.api.toDataFrame import org.jetbrains.kotlinx.dataframe.io.DisplayConfiguration import org.jetbrains.kotlinx.dataframe.io.escapeHTML @@ -174,4 +177,20 @@ class RenderingTests : TestBase() { dfGroup.name.lastName.maxWidth() shouldBe 1 dfGroup.name.firstName.secondName.maxWidth() shouldBe 1 } + + @Test + fun `render array types correctly`() { + val df = dataFrameOf( + columnOf(1, null).named("a"), + columnOf(intArrayOf(1), intArrayOf(2)).named("b"), + // TODO https://github.com/Kotlin/dataframe/issues/679 + // columnOf(arrayOf(1), arrayOf(2)).named("d"), + DataColumn.createValueColumn("c", listOf(arrayOf(1), arrayOf(2))), + columnOf(arrayOf(1, null), arrayOf(2, null)).named("d"), + ) + + val schema = df.schema() + val rendered = schema.toString() + rendered shouldBe "a: Int?\nb: IntArray\nc: Array\nd: Array" + } } From 65a3e026fde44c8149ebc5b76048d3d7c66f37b8 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 25 Apr 2024 16:40:14 +0200 Subject: [PATCH 09/13] lint fixes --- .../kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt | 1 - .../kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt | 1 - .../org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt | 1 - .../kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt | 1 - .../kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt | 1 - .../org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt | 1 - 6 files changed, 6 deletions(-) diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt index 1326285b8..f463836a5 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt @@ -59,7 +59,6 @@ internal fun renderType(type: KType?): String { else -> { val fullName = type.jvmErasure.qualifiedName ?: return type.toString() val name = when { - // catching cases like `typeOf>().jvmErasure.qualifiedName == "IntArray"` // https://github.com/Kotlin/dataframe/issues/678 type.isSubtypeOf(typeOf>()) -> diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index 0214a2bce..3200684ec 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -519,7 +519,6 @@ internal val KType.isPrimitiveArray: Boolean (classifier as? KClass<*>)?.isPrimitiveArray == true } - /** * Returns `true` if this type is of an array, either a primitive array like `XArray` or `Array<>`. * 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 8586c7300..958463e6d 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 @@ -335,7 +335,6 @@ internal fun convertToDataFrame( } } - else -> { val df = convertToDataFrame( data = values, diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt index 1326285b8..f463836a5 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/Rendering.kt @@ -59,7 +59,6 @@ internal fun renderType(type: KType?): String { else -> { val fullName = type.jvmErasure.qualifiedName ?: return type.toString() val name = when { - // catching cases like `typeOf>().jvmErasure.qualifiedName == "IntArray"` // https://github.com/Kotlin/dataframe/issues/678 type.isSubtypeOf(typeOf>()) -> diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index 0214a2bce..3200684ec 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -519,7 +519,6 @@ internal val KType.isPrimitiveArray: Boolean (classifier as? KClass<*>)?.isPrimitiveArray == true } - /** * Returns `true` if this type is of an array, either a primitive array like `XArray` or `Array<>`. * 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 8586c7300..958463e6d 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 @@ -335,7 +335,6 @@ internal fun convertToDataFrame( } } - else -> { val df = convertToDataFrame( data = values, From 23a7469b2a3804e354aa629ed8790570b5ebfc69 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 25 Apr 2024 17:37:40 +0200 Subject: [PATCH 10/13] typo --- .../kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt | 6 +++--- .../kotlin/org/jetbrains/kotlinx/dataframe/impl/Utils.kt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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 5f1241a21..c82394b6d 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 @@ -370,7 +370,7 @@ internal fun KCallable<*>.isGetterLike(): Boolean = /** Returns the column name for this callable. * If the callable contains the [ColumnName][org.jetbrains.kotlinx.dataframe.annotations.ColumnName] annotation, its [ColumnName.name][org.jetbrains.kotlinx.dataframe.annotations.ColumnName.name] is returned. - * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. */ + * Otherwise, the name of the callable is returned with proper getter-trimming if it's a [KFunction]. */ @PublishedApi internal val KFunction<*>.columnName: String get() = findAnnotation()?.name @@ -381,7 +381,7 @@ internal val KFunction<*>.columnName: String /** Returns the column name for this callable. * If the callable contains the [ColumnName][org.jetbrains.kotlinx.dataframe.annotations.ColumnName] annotation, its [ColumnName.name][org.jetbrains.kotlinx.dataframe.annotations.ColumnName.name] is returned. - * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. */ + * Otherwise, the name of the callable is returned with proper getter-trimming if it's a [KFunction]. */ @PublishedApi internal val KProperty<*>.columnName: String get() = findAnnotation()?.name ?: name @@ -389,7 +389,7 @@ internal val KProperty<*>.columnName: String /** * Returns the column name for this callable. * If the callable contains the [ColumnName] annotation, its [ColumnName.name] is returned. - * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. + * Otherwise, the name of the callable is returned with proper getter-trimming if it's a [KFunction]. */ @PublishedApi internal val KCallable<*>.columnName: String 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 7ee866914..6cb461271 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 @@ -379,7 +379,7 @@ internal val KProperty<*>.columnName: String /** * Returns the column name for this callable. * If the callable contains the [ColumnName] annotation, its [ColumnName.name] is returned. - * Otherwise, the name of the callable is returned with proper getter-trimming iff it's a [KFunction]. + * Otherwise, the name of the callable is returned with proper getter-trimming if it's a [KFunction]. */ @PublishedApi internal val KCallable<*>.columnName: String From a46d3ad05c7cfca3168b06931df14380df6ebbc9 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Fri, 26 Apr 2024 14:02:30 +0200 Subject: [PATCH 11/13] asArrayAsListOrNull with asList --- .../kotlinx/dataframe/impl/TypeUtils.kt | 43 +++++++++---------- .../jetbrains/kotlinx/dataframe/io/string.kt | 11 ++--- .../kotlinx/dataframe/api/toDataFrame.kt | 8 ++-- .../kotlinx/dataframe/types/UtilTests.kt | 13 +++--- .../kotlinx/dataframe/impl/TypeUtils.kt | 43 +++++++++---------- .../jetbrains/kotlinx/dataframe/io/string.kt | 11 ++--- .../kotlinx/dataframe/api/toDataFrame.kt | 8 ++-- .../kotlinx/dataframe/types/UtilTests.kt | 13 +++--- 8 files changed, 70 insertions(+), 80 deletions(-) diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index 3200684ec..139bdb177 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -544,28 +544,25 @@ internal val Any.isArray: Boolean get() = this::class.isArray /** - * Returns this array object as a list of values. - * - * @throws IllegalArgumentException if this object is not an array + * If [this] is an array of any kind, the function returns it as a list of values, + * else it returns `null`. */ -internal fun Any.asArrayToList(): List<*> { - require(this.isArray) { "Not an array" } - return when (this) { - is BooleanArray -> toList() - is ByteArray -> toList() - is ShortArray -> toList() - is IntArray -> toList() - is LongArray -> toList() - is FloatArray -> toList() - is DoubleArray -> toList() - is CharArray -> toList() - - is UByteArray -> toList() - is UShortArray -> toList() - is UIntArray -> toList() - is ULongArray -> toList() - - is Array<*> -> toList() - else -> error("Not an array") +internal fun Any.asArrayAsListOrNull(): List<*>? = + when (this) { + is BooleanArray -> asList() + is ByteArray -> asList() + is ShortArray -> asList() + is IntArray -> asList() + is LongArray -> asList() + is FloatArray -> asList() + is DoubleArray -> asList() + is CharArray -> asList() + + is UByteArray -> asList() + is UShortArray -> asList() + is UIntArray -> asList() + is ULongArray -> asList() + + is Array<*> -> asList() + else -> null } -} diff --git a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt index a69c61322..da54d4124 100644 --- a/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt +++ b/core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt @@ -7,8 +7,7 @@ import org.jetbrains.kotlinx.dataframe.api.columnsCount import org.jetbrains.kotlinx.dataframe.api.isNumber import org.jetbrains.kotlinx.dataframe.api.take import org.jetbrains.kotlinx.dataframe.api.toColumn -import org.jetbrains.kotlinx.dataframe.impl.asArrayToList -import org.jetbrains.kotlinx.dataframe.impl.isArray +import org.jetbrains.kotlinx.dataframe.impl.asArrayAsListOrNull import org.jetbrains.kotlinx.dataframe.impl.owner import org.jetbrains.kotlinx.dataframe.impl.renderType import org.jetbrains.kotlinx.dataframe.impl.scale @@ -177,11 +176,9 @@ internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalForm is List<*> -> if (value.isEmpty()) "[ ]" else value.toString() is Array<*> -> if (value.isEmpty()) "[ ]" else value.toList().toString() else -> - if (value?.isArray == true) { - renderValueToString(value.asArrayToList(), decimalFormat) - } else { - value.toString() - } + value + ?.asArrayAsListOrNull()?.let { renderValueToString(it, decimalFormat) } + ?: value.toString() } internal fun internallyRenderable(value: Any?): Boolean { diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index fba1ffdb7..bf5aff31d 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -220,16 +220,16 @@ class CreateDataFrameTests { @Test fun builtInTypes() { val string = listOf("aaa", "aa", null) - string.toDataFrame().also { it.print() } shouldBe dataFrameOf("value")(*string.toTypedArray()) + string.toDataFrame() shouldBe dataFrameOf("value")(*string.toTypedArray()) val int = listOf(1, 2, 3) - int.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*int.toTypedArray()) + int.toDataFrame() shouldBe dataFrameOf("value")(*int.toTypedArray()) val doubles = listOf(1.0, 2.0, 3.0) - doubles.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*doubles.toTypedArray()) + doubles.toDataFrame() shouldBe dataFrameOf("value")(*doubles.toTypedArray()) val floats = listOf(1.0f, 2.0f, 3.0f) - floats.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*floats.toTypedArray()) + floats.toDataFrame() shouldBe dataFrameOf("value")(*floats.toTypedArray()) } @Ignore diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt index e909d9fec..3b3f33b80 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt @@ -1,7 +1,7 @@ package org.jetbrains.kotlinx.dataframe.types import io.kotest.matchers.shouldBe -import org.jetbrains.kotlinx.dataframe.impl.asArrayToList +import org.jetbrains.kotlinx.dataframe.impl.asArrayAsListOrNull import org.jetbrains.kotlinx.dataframe.impl.commonParent import org.jetbrains.kotlinx.dataframe.impl.commonParents import org.jetbrains.kotlinx.dataframe.impl.commonType @@ -62,11 +62,12 @@ class UtilTests { arrayOfNulls(1).isPrimitiveArray shouldBe false // Any asArrayToList - booleanArrayOf(true, false).asArrayToList() shouldBe listOf(true, false) - uintArrayOf(1u, 2u).asArrayToList() shouldBe listOf(1u, 2u) - arrayOf(1, 2).asArrayToList() shouldBe listOf(1, 2) - arrayOf(1, null).asArrayToList() shouldBe listOf(1, null) - arrayOfNulls(1).asArrayToList() shouldBe listOf(null) + booleanArrayOf(true, false).asArrayAsListOrNull() shouldBe listOf(true, false) + uintArrayOf(1u, 2u).asArrayAsListOrNull() shouldBe listOf(1u, 2u) + arrayOf(1, 2).asArrayAsListOrNull() shouldBe listOf(1, 2) + arrayOf(1, null).asArrayAsListOrNull() shouldBe listOf(1, null) + arrayOfNulls(1).asArrayAsListOrNull() shouldBe listOf(null) + 1.asArrayAsListOrNull() shouldBe null } @Test diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt index 3200684ec..139bdb177 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/TypeUtils.kt @@ -544,28 +544,25 @@ internal val Any.isArray: Boolean get() = this::class.isArray /** - * Returns this array object as a list of values. - * - * @throws IllegalArgumentException if this object is not an array + * If [this] is an array of any kind, the function returns it as a list of values, + * else it returns `null`. */ -internal fun Any.asArrayToList(): List<*> { - require(this.isArray) { "Not an array" } - return when (this) { - is BooleanArray -> toList() - is ByteArray -> toList() - is ShortArray -> toList() - is IntArray -> toList() - is LongArray -> toList() - is FloatArray -> toList() - is DoubleArray -> toList() - is CharArray -> toList() - - is UByteArray -> toList() - is UShortArray -> toList() - is UIntArray -> toList() - is ULongArray -> toList() - - is Array<*> -> toList() - else -> error("Not an array") +internal fun Any.asArrayAsListOrNull(): List<*>? = + when (this) { + is BooleanArray -> asList() + is ByteArray -> asList() + is ShortArray -> asList() + is IntArray -> asList() + is LongArray -> asList() + is FloatArray -> asList() + is DoubleArray -> asList() + is CharArray -> asList() + + is UByteArray -> asList() + is UShortArray -> asList() + is UIntArray -> asList() + is ULongArray -> asList() + + is Array<*> -> asList() + else -> null } -} diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt index a69c61322..da54d4124 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/string.kt @@ -7,8 +7,7 @@ import org.jetbrains.kotlinx.dataframe.api.columnsCount import org.jetbrains.kotlinx.dataframe.api.isNumber import org.jetbrains.kotlinx.dataframe.api.take import org.jetbrains.kotlinx.dataframe.api.toColumn -import org.jetbrains.kotlinx.dataframe.impl.asArrayToList -import org.jetbrains.kotlinx.dataframe.impl.isArray +import org.jetbrains.kotlinx.dataframe.impl.asArrayAsListOrNull import org.jetbrains.kotlinx.dataframe.impl.owner import org.jetbrains.kotlinx.dataframe.impl.renderType import org.jetbrains.kotlinx.dataframe.impl.scale @@ -177,11 +176,9 @@ internal fun renderValueToString(value: Any?, decimalFormat: RendererDecimalForm is List<*> -> if (value.isEmpty()) "[ ]" else value.toString() is Array<*> -> if (value.isEmpty()) "[ ]" else value.toList().toString() else -> - if (value?.isArray == true) { - renderValueToString(value.asArrayToList(), decimalFormat) - } else { - value.toString() - } + value + ?.asArrayAsListOrNull()?.let { renderValueToString(it, decimalFormat) } + ?: value.toString() } internal fun internallyRenderable(value: Any?): Boolean { diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index fba1ffdb7..bf5aff31d 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -220,16 +220,16 @@ class CreateDataFrameTests { @Test fun builtInTypes() { val string = listOf("aaa", "aa", null) - string.toDataFrame().also { it.print() } shouldBe dataFrameOf("value")(*string.toTypedArray()) + string.toDataFrame() shouldBe dataFrameOf("value")(*string.toTypedArray()) val int = listOf(1, 2, 3) - int.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*int.toTypedArray()) + int.toDataFrame() shouldBe dataFrameOf("value")(*int.toTypedArray()) val doubles = listOf(1.0, 2.0, 3.0) - doubles.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*doubles.toTypedArray()) + doubles.toDataFrame() shouldBe dataFrameOf("value")(*doubles.toTypedArray()) val floats = listOf(1.0f, 2.0f, 3.0f) - floats.toDataFrame().alsoDebug() shouldBe dataFrameOf("value")(*floats.toTypedArray()) + floats.toDataFrame() shouldBe dataFrameOf("value")(*floats.toTypedArray()) } @Ignore diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt index e909d9fec..3b3f33b80 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/types/UtilTests.kt @@ -1,7 +1,7 @@ package org.jetbrains.kotlinx.dataframe.types import io.kotest.matchers.shouldBe -import org.jetbrains.kotlinx.dataframe.impl.asArrayToList +import org.jetbrains.kotlinx.dataframe.impl.asArrayAsListOrNull import org.jetbrains.kotlinx.dataframe.impl.commonParent import org.jetbrains.kotlinx.dataframe.impl.commonParents import org.jetbrains.kotlinx.dataframe.impl.commonType @@ -62,11 +62,12 @@ class UtilTests { arrayOfNulls(1).isPrimitiveArray shouldBe false // Any asArrayToList - booleanArrayOf(true, false).asArrayToList() shouldBe listOf(true, false) - uintArrayOf(1u, 2u).asArrayToList() shouldBe listOf(1u, 2u) - arrayOf(1, 2).asArrayToList() shouldBe listOf(1, 2) - arrayOf(1, null).asArrayToList() shouldBe listOf(1, null) - arrayOfNulls(1).asArrayToList() shouldBe listOf(null) + booleanArrayOf(true, false).asArrayAsListOrNull() shouldBe listOf(true, false) + uintArrayOf(1u, 2u).asArrayAsListOrNull() shouldBe listOf(1u, 2u) + arrayOf(1, 2).asArrayAsListOrNull() shouldBe listOf(1, 2) + arrayOf(1, null).asArrayAsListOrNull() shouldBe listOf(1, null) + arrayOfNulls(1).asArrayAsListOrNull() shouldBe listOf(null) + 1.asArrayAsListOrNull() shouldBe null } @Test From 07d933bc8987372d9bc74595a76295502ed565a6 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Fri, 26 Apr 2024 15:45:35 +0200 Subject: [PATCH 12/13] fixed test --- .../kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt | 2 +- .../kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index bf5aff31d..90d427bd9 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -382,7 +382,7 @@ class CreateDataFrameTests { listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe - dataFrameOf("b")("bb").groupBy("").concat() + dataFrameOf("b")("bb") listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getA) diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index bf5aff31d..90d427bd9 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -382,7 +382,7 @@ class CreateDataFrameTests { listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getA) } shouldBe dataFrameOf("a")(1) listOf(KotlinPojo("bb", 1)).toDataFrame { properties(KotlinPojo::getB) } shouldBe - dataFrameOf("b")("bb").groupBy("").concat() + dataFrameOf("b")("bb") listOf(JavaPojo(2.0, 3, "bb", 1)).toDataFrame { properties(JavaPojo::getA) From fca4b8fd97b2219f899202b379d586107b5152d8 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Mon, 29 Apr 2024 12:54:17 +0200 Subject: [PATCH 13/13] simplified sorting by constructor --- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 6 +-- .../kotlinx/dataframe/impl/schema/Utils.kt | 49 +++++++------------ .../kotlinx/dataframe/api/toDataFrame.kt | 10 ---- .../kotlinx/dataframe/impl/api/toDataFrame.kt | 6 +-- .../kotlinx/dataframe/impl/schema/Utils.kt | 49 +++++++------------ .../kotlinx/dataframe/api/toDataFrame.kt | 10 ---- 6 files changed, 40 insertions(+), 90 deletions(-) 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 958463e6d..9c9bc6da3 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 @@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType import org.jetbrains.kotlinx.dataframe.impl.isArray import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors +import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal @@ -178,8 +178,8 @@ internal fun convertToDataFrame( .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() } } - // sort properties by order in constructors - .sortWithConstructors(clazz) + // sort properties by order in constructor + .sortWithConstructor(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 9400e02b1..b083c5ab2 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 @@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame = internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame = MarkersExtractor.get(clazz).schema.createEmptyDataFrame() +/** + * Returns a map of property names to their order in the primary/single constructor, if it exists, + * `null` otherwise. + */ internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map? = - clazz.primaryConstructor + (clazz.primaryConstructor ?: clazz.constructors.singleOrNull()) ?.parameters ?.mapNotNull { it.name } ?.mapIndexed { i, v -> v to i } ?.toMap() -internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List> = - clazz.constructors - .map { constructor -> - constructor.parameters - .mapNotNull { it.name } - .mapIndexed { i, v -> 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. + * Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass] + * if it exists, else, it falls back to lexicographical sorting. */ -internal fun Iterable>.sortWithConstructors(klass: KClass<*>): List> { +internal fun Iterable>.sortWithConstructor(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() + // starting off lexicographically + val lexicographicalColumns = sortedBy { it.columnName } + // if no primary constructor, return lexicographical order if (primaryConstructorOrder == null) { - return allConstructorsSortedProperties + return lexicographicalColumns } - // 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 - + // 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) = - this.partition { it.columnName in primaryConstructorOrder.keys } - - val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName } + lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys } - return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } + - propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) } + return propsInConstructor + .sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor } diff --git a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 90d427bd9..21ec8c964 100644 --- a/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -319,21 +319,11 @@ class CreateDataFrameTests { private var a: Int = 0 private var b: String = "" - constructor() - constructor(b: String, a: Int) { this.a = a this.b = b } - constructor(b: String) { - this.b = b - } - - constructor(a: Int) { - this.a = a - } - fun getA(): Int = a fun setA(a: Int) { this.a = a 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 958463e6d..9c9bc6da3 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 @@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType import org.jetbrains.kotlinx.dataframe.impl.isArray import org.jetbrains.kotlinx.dataframe.impl.isGetterLike import org.jetbrains.kotlinx.dataframe.impl.projectUpTo -import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors +import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method import java.time.temporal.Temporal @@ -178,8 +178,8 @@ internal fun convertToDataFrame( .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() } } - // sort properties by order in constructors - .sortWithConstructors(clazz) + // sort properties by order in constructor + .sortWithConstructor(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 9400e02b1..b083c5ab2 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 @@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame = internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame = MarkersExtractor.get(clazz).schema.createEmptyDataFrame() +/** + * Returns a map of property names to their order in the primary/single constructor, if it exists, + * `null` otherwise. + */ internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map? = - clazz.primaryConstructor + (clazz.primaryConstructor ?: clazz.constructors.singleOrNull()) ?.parameters ?.mapNotNull { it.name } ?.mapIndexed { i, v -> v to i } ?.toMap() -internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List> = - clazz.constructors - .map { constructor -> - constructor.parameters - .mapNotNull { it.name } - .mapIndexed { i, v -> 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. + * Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass] + * if it exists, else, it falls back to lexicographical sorting. */ -internal fun Iterable>.sortWithConstructors(klass: KClass<*>): List> { +internal fun Iterable>.sortWithConstructor(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() + // starting off lexicographically + val lexicographicalColumns = sortedBy { it.columnName } + // if no primary constructor, return lexicographical order if (primaryConstructorOrder == null) { - return allConstructorsSortedProperties + return lexicographicalColumns } - // 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 - + // 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) = - this.partition { it.columnName in primaryConstructorOrder.keys } - - val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName } + lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys } - return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } + - propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) } + return propsInConstructor + .sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor } diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt index 90d427bd9..21ec8c964 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt @@ -319,21 +319,11 @@ class CreateDataFrameTests { private var a: Int = 0 private var b: String = "" - constructor() - constructor(b: String, a: Int) { this.a = a this.b = b } - constructor(b: String) { - this.b = b - } - - constructor(a: Int) { - this.a = a - } - fun getA(): Int = a fun setA(a: Int) { this.a = a