From a032e824cf476338e6154262cc8b8bc334e6dae3 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Wed, 7 Dec 2022 17:07:16 +0100 Subject: [PATCH 1/4] removed the requirement of dataframe properties to be mutable for isOpen to be true in DataSchemas in Jupyter --- .../dataframe/codeGen/ReplCodeGenerator.kt | 3 --- .../impl/codeGen/ReplCodeGeneratorImpl.kt | 14 +++++------- .../dataframe/codeGen/ReplCodeGenTests.kt | 22 +++++++++---------- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenerator.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenerator.kt index 92ded3cf3..63ceb4ffd 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenerator.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenerator.kt @@ -6,7 +6,6 @@ import org.jetbrains.kotlinx.dataframe.codeGen.CodeWithConverter import org.jetbrains.kotlinx.dataframe.impl.codeGen.ReplCodeGeneratorImpl import org.jetbrains.kotlinx.jupyter.api.Code import kotlin.reflect.KClass -import kotlin.reflect.KMutableProperty import kotlin.reflect.KProperty internal interface ReplCodeGenerator { @@ -14,13 +13,11 @@ internal interface ReplCodeGenerator { fun process( df: AnyFrame, property: KProperty<*>? = null, - isMutable: Boolean = property is KMutableProperty?, ): CodeWithConverter fun process( row: AnyRow, property: KProperty<*>? = null, - isMutable: Boolean = property is KMutableProperty?, ): CodeWithConverter fun process(markerClass: KClass<*>): Code diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/codeGen/ReplCodeGeneratorImpl.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/codeGen/ReplCodeGeneratorImpl.kt index 815cf4048..ee32c8e23 100644 --- a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/codeGen/ReplCodeGeneratorImpl.kt +++ b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/codeGen/ReplCodeGeneratorImpl.kt @@ -16,7 +16,6 @@ import org.jetbrains.kotlinx.dataframe.codeGen.MarkersExtractor import org.jetbrains.kotlinx.dataframe.schema.DataFrameSchema import org.jetbrains.kotlinx.jupyter.api.Code import kotlin.reflect.KClass -import kotlin.reflect.KMutableProperty import kotlin.reflect.KProperty import kotlin.reflect.KType import kotlin.reflect.KVisibility @@ -45,25 +44,22 @@ internal class ReplCodeGeneratorImpl : ReplCodeGenerator { else -> null } - override fun process(row: AnyRow, property: KProperty<*>?, isMutable: Boolean) = - process(row.df(), property, isMutable) + override fun process(row: AnyRow, property: KProperty<*>?): CodeWithConverter = process(row.df(), property) - override fun process(df: AnyFrame, property: KProperty<*>?, isMutable: Boolean): CodeWithConverter { + override fun process(df: AnyFrame, property: KProperty<*>?): CodeWithConverter { var targetSchema = df.schema() - var isMutable = isMutable if (property != null) { val wasProcessedBefore = property in registeredProperties registeredProperties.add(property) - isMutable = property is KMutableProperty // maybe property is already properly typed, let's do some checks val currentMarker = getMarkerClass(property.returnType) ?.takeIf { it.findAnnotation() != null } ?.let { registeredMarkers[it] ?: MarkersExtractor.get(it) } if (currentMarker != null) { - // if property is mutable, we need to make sure that its marker type is open in order to let derived data frames be assignable to it - if (!isMutable || currentMarker.isOpen) { + // we need to make sure that the property's marker type is open in order to let derived data frames be assignable to it + if (currentMarker.isOpen) { val columnSchema = currentMarker.schema // for mutable properties we do strong typing only at the first processing, after that we allow its type to be more general than actual data frame type if (wasProcessedBefore || columnSchema == targetSchema) { @@ -79,7 +75,7 @@ internal class ReplCodeGeneratorImpl : ReplCodeGenerator { } } - return generate(schema = targetSchema, name = markerInterfacePrefix, isOpen = isMutable) + return generate(schema = targetSchema, name = markerInterfacePrefix, isOpen = true) } fun generate( diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt index 40b7f4141..645db8dcc 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt @@ -68,7 +68,7 @@ class ReplCodeGenTests : BaseTest() { @Test fun `process derived markers`() { val repl = ReplCodeGenerator.create() - val code = repl.process(df, isMutable = true).declarations + val code = repl.process(df).declarations val marker = ReplCodeGeneratorImpl.markerInterfacePrefix val markerFull = Test1._DataFrameType::class.qualifiedName!! @@ -100,10 +100,10 @@ class ReplCodeGenTests : BaseTest() { code2 shouldBe "" val df3 = typed.filter { city != null } - val code3 = repl.process(df3, isMutable = false).declarations + val code3 = repl.process(df3).declarations val marker3 = marker + "1" val expected3 = """ - @DataSchema(isOpen = false) + @DataSchema interface $marker3 : $markerFull val $dfName<$marker3>.city: $dataCol<$stringName> @JvmName("${marker3}_city") get() = this["city"] as $dataCol<$stringName> @@ -118,10 +118,10 @@ class ReplCodeGenTests : BaseTest() { code4 shouldBe "" val df5 = typed.filter { weight != null } - val code5 = repl.process(df5, isMutable = false).declarations + val code5 = repl.process(df5).declarations val marker5 = marker + "2" val expected5 = """ - @DataSchema(isOpen = false) + @DataSchema interface $marker5 : $markerFull val $dfName<$marker5>.weight: $dataCol<$intName> @JvmName("${marker5}_weight") get() = this["weight"] as $dataCol<$intName> @@ -144,12 +144,12 @@ class ReplCodeGenTests : BaseTest() { repl.process() shouldBe "" val expected = """ - @DataSchema(isOpen = false) + @DataSchema interface ${Test2._DataFrameType2::class.simpleName!!} : ${Test2._DataFrameType::class.qualifiedName}, ${Test2._DataFrameType1::class.qualifiedName} """.trimIndent() - val code = repl.process(typed, isMutable = false).declarations.trimIndent() + val code = repl.process(typed).declarations.trimIndent() code shouldBe expected } @@ -163,7 +163,7 @@ class ReplCodeGenTests : BaseTest() { val marker = Test2._DataFrameType2::class.simpleName!! val expected = """ - @DataSchema(isOpen = false) + @DataSchema interface $marker : ${Test2._DataFrameType::class.qualifiedName} val $dfName<$marker>.city: $dataCol<$stringName?> @JvmName("${marker}_city") get() = this["city"] as $dataCol<$stringName?> @@ -176,17 +176,17 @@ class ReplCodeGenTests : BaseTest() { val $dfRowName<$marker?>.weight: $intName? @JvmName("Nullable${marker}_weight") get() = this["weight"] as $intName? """.trimIndent() - val code = repl.process(typed, isMutable = false).declarations.trimIndent() + val code = repl.process(typed).declarations.trimIndent() code shouldBe expected } @Test - fun `process overriden property`() { + fun `process overridden property`() { val repl = ReplCodeGenerator.create() repl.process() repl.process() repl.process() - val c = repl.process(Test3.df, Test3::df) + val c = repl.process(Test3.df, Test3::df) // TODO this now generates stuff c.declarations.shouldBeEmpty() } From 089e8027e95a88ead2980792687684edf6b1efd5 Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Thu, 8 Dec 2022 14:22:11 +0100 Subject: [PATCH 2/4] fixed tests --- .../kotlinx/dataframe/codeGen/CodeGenerationTests.kt | 6 +++--- .../jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/CodeGenerationTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/CodeGenerationTests.kt index 0b2bf338d..70c09b5d8 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/CodeGenerationTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/CodeGenerationTests.kt @@ -59,7 +59,7 @@ class CodeGenerationTests : BaseTest() { val generated = codeGen.process(df, ::df) val typeName = ReplCodeGeneratorImpl.markerInterfacePrefix val expectedDeclaration = """ - @DataSchema(isOpen = false) + @DataSchema interface $typeName """.trimIndent() + "\n" + expectedProperties(typeName, typeName) @@ -84,7 +84,7 @@ class CodeGenerationTests : BaseTest() { val generated = ReplCodeGenerator.create().process(df[0], property) val typeName = ReplCodeGeneratorImpl.markerInterfacePrefix val expectedDeclaration = """ - @DataSchema(isOpen = false) + @DataSchema interface $typeName """.trimIndent() + "\n" + expectedProperties(typeName, typeName) @@ -118,7 +118,7 @@ class CodeGenerationTests : BaseTest() { """.trimIndent() val declaration2 = """ - @DataSchema(isOpen = false) + @DataSchema interface $type2 val $dfName<$type2>.age: $dataCol<$intName> @JvmName("${type2}_age") get() = this["age"] as $dataCol<$intName> diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt index 645db8dcc..50a3fbbf2 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/codeGen/ReplCodeGenTests.kt @@ -1,7 +1,7 @@ package org.jetbrains.kotlinx.dataframe.codeGen import io.kotest.matchers.shouldBe -import io.kotest.matchers.string.shouldBeEmpty +import io.kotest.matchers.string.shouldNotBeEmpty import org.jetbrains.dataframe.impl.codeGen.ReplCodeGenerator import org.jetbrains.dataframe.impl.codeGen.process import org.jetbrains.kotlinx.dataframe.ColumnsContainer @@ -186,8 +186,8 @@ class ReplCodeGenTests : BaseTest() { repl.process() repl.process() repl.process() - val c = repl.process(Test3.df, Test3::df) // TODO this now generates stuff - c.declarations.shouldBeEmpty() + val c = repl.process(Test3.df, Test3::df) + c.declarations.shouldNotBeEmpty() } @Test From 00936592b5207c831ecca4ccbef02b18908bc1bd Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Mon, 5 Dec 2022 17:21:27 +0100 Subject: [PATCH 3/4] test and bugfix for convertTo etc. not recognizing super properties correctly --- .../dataframe/codeGen/MarkersExtractor.kt | 4 ++-- .../kotlinx/dataframe/api/convertTo.kt | 21 +++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) 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 37f5e3947..ef883a33e 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 @@ -8,9 +8,9 @@ import org.jetbrains.kotlinx.dataframe.impl.schema.getPropertiesOrder import org.jetbrains.kotlinx.dataframe.schema.ColumnSchema import kotlin.reflect.KClass import kotlin.reflect.KType -import kotlin.reflect.full.declaredMemberProperties import kotlin.reflect.full.findAnnotation import kotlin.reflect.full.hasAnnotation +import kotlin.reflect.full.memberProperties import kotlin.reflect.full.superclasses import kotlin.reflect.full.withNullability import kotlin.reflect.jvm.jvmErasure @@ -54,7 +54,7 @@ internal object MarkersExtractor { private fun getFields(markerClass: KClass<*>, nullableProperties: Boolean): List { val order = getPropertiesOrder(markerClass) - return markerClass.declaredMemberProperties.sortedBy { order[it.name] ?: Int.MAX_VALUE }.mapIndexed { _, it -> + 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 val type = it.returnType diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/convertTo.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/convertTo.kt index 2fe735686..fb9ffe3a4 100644 --- a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/convertTo.kt +++ b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/convertTo.kt @@ -4,6 +4,7 @@ import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldBe import org.jetbrains.kotlinx.dataframe.AnyFrame import org.jetbrains.kotlinx.dataframe.DataFrame +import org.jetbrains.kotlinx.dataframe.alsoDebug import org.jetbrains.kotlinx.dataframe.annotations.DataSchema import org.jetbrains.kotlinx.dataframe.exceptions.TypeConverterNotFoundException import org.junit.Test @@ -239,9 +240,21 @@ class ConvertToTests { .alsoDebug("df5 after second convert:") } - private fun > T.alsoDebug(println: String? = null): T = apply { - println?.let { println(it) } - print(borders = true, title = true, columnTypes = true, valueLimit = -1) - schema().print() + interface KeyValue { + val key: String + val value: T + } + + @DataSchema + interface MySchema : KeyValue + + @Test + fun `Convert generic interface to itself`() { + val df = dataFrameOf("key", "value")( + "a", 1, + "b", 2, + ).alsoDebug() + val converted = df.convertTo().alsoDebug() + converted shouldBe df } } From a15e1f041b30419762316672a320c589db530a2e Mon Sep 17 00:00:00 2001 From: Jolan Rensen Date: Mon, 5 Dec 2022 17:50:44 +0100 Subject: [PATCH 4/4] fixing same issue for extension generation by ksp with test --- .../dataframe/ksp/ExtensionsGenerator.kt | 8 ++--- .../ksp/DataFrameSymbolProcessorTest.kt | 31 +++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/plugins/symbol-processor/src/main/kotlin/org/jetbrains/dataframe/ksp/ExtensionsGenerator.kt b/plugins/symbol-processor/src/main/kotlin/org/jetbrains/dataframe/ksp/ExtensionsGenerator.kt index ca9fa8cb1..6f705d679 100644 --- a/plugins/symbol-processor/src/main/kotlin/org/jetbrains/dataframe/ksp/ExtensionsGenerator.kt +++ b/plugins/symbol-processor/src/main/kotlin/org/jetbrains/dataframe/ksp/ExtensionsGenerator.kt @@ -12,7 +12,6 @@ import com.google.devtools.ksp.symbol.KSClassifierReference import com.google.devtools.ksp.symbol.KSDeclaration import com.google.devtools.ksp.symbol.KSFile import com.google.devtools.ksp.symbol.KSName -import com.google.devtools.ksp.symbol.KSPropertyDeclaration import com.google.devtools.ksp.symbol.KSTypeReference import com.google.devtools.ksp.symbol.KSValueArgument import com.google.devtools.ksp.symbol.Modifier @@ -61,11 +60,10 @@ class ExtensionsGenerator( return when { isClassOrInterface() && effectivelyPublicOrInternal() -> { DataSchemaDeclaration( - this, - declarations - .filterIsInstance() + origin = this, + properties = getAllProperties() .map { KSAnnotatedWithType(it, it.simpleName, it.type) } - .toList() + .toList(), ) } else -> null diff --git a/plugins/symbol-processor/src/test/kotlin/org/jetbrains/dataframe/ksp/DataFrameSymbolProcessorTest.kt b/plugins/symbol-processor/src/test/kotlin/org/jetbrains/dataframe/ksp/DataFrameSymbolProcessorTest.kt index 5fcf0997e..a9dcac51a 100644 --- a/plugins/symbol-processor/src/test/kotlin/org/jetbrains/dataframe/ksp/DataFrameSymbolProcessorTest.kt +++ b/plugins/symbol-processor/src/test/kotlin/org/jetbrains/dataframe/ksp/DataFrameSymbolProcessorTest.kt @@ -748,6 +748,37 @@ class DataFrameSymbolProcessorTest { result.successfulCompilation shouldBe true } + @Test + fun `generic interface as supertype`() { + val result = KspCompilationTestRunner.compile( + TestCompilationParameters( + sources = listOf( + SourceFile.kotlin( + "MySources.kt", + """ + package org.example + + $imports + + interface KeyValue { + val key: String + val value: T + } + + @DataSchema + interface MySchema : KeyValue + + + val ColumnsContainer.test1: DataColumn get() = key + val DataRow.test2: Int get() = value + """.trimIndent() + ) + ) + ) + ) + result.successfulCompilation shouldBe true + } + @Test fun `nested interface`() { val result = KspCompilationTestRunner.compile(