Fix #640: Jupyter integration conflicts with variable type converters…

… from other integrations.
Kotlin · Apr 15, 2024 · 195a305 · 195a305
1 parent 9de12a3
commit 195a305
Show file tree

Hide file tree

Showing 4 changed files with 74 additions and 36 deletions.
diff --git a/.../generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/Integration.kt b/.../generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/Integration.kt
@@ -45,15 +45,8 @@ import org.jetbrains.kotlinx.dataframe.impl.renderType
 import org.jetbrains.kotlinx.dataframe.io.DataFrameHtmlData
 import org.jetbrains.kotlinx.dataframe.io.SupportedCodeGenerationFormat
 import org.jetbrains.kotlinx.dataframe.io.supportedFormats
-import org.jetbrains.kotlinx.jupyter.api.HTML
-import org.jetbrains.kotlinx.jupyter.api.JupyterClientType
-import org.jetbrains.kotlinx.jupyter.api.KotlinKernelHost
-import org.jetbrains.kotlinx.jupyter.api.Notebook
-import org.jetbrains.kotlinx.jupyter.api.VariableName
-import org.jetbrains.kotlinx.jupyter.api.declare
-import org.jetbrains.kotlinx.jupyter.api.libraries.ColorScheme
-import org.jetbrains.kotlinx.jupyter.api.libraries.JupyterIntegration
-import org.jetbrains.kotlinx.jupyter.api.libraries.resources
+import org.jetbrains.kotlinx.jupyter.api.*
+import org.jetbrains.kotlinx.jupyter.api.libraries.*
 import kotlin.reflect.KClass
 import kotlin.reflect.KProperty
 import kotlin.reflect.KType
@@ -281,16 +274,25 @@ internal class Integration(
         import("org.jetbrains.kotlinx.dataframe.dataTypes.*")
         import("org.jetbrains.kotlinx.dataframe.impl.codeGen.urlCodeGenReader")
 
-        updateVariable<Any> { instance, property ->
-            when (instance) {
-                is AnyCol -> updateAnyColVariable(instance, property, codeGen)
-                is ColumnGroup<*> -> updateColumnGroupVariable(instance, property, codeGen)
-                is AnyRow -> updateAnyRowVariable(instance, property, codeGen)
-                is AnyFrame -> updateAnyFrameVariable(instance, property, codeGen)
-                is ImportDataSchema -> updateImportDataSchemaVariable(instance, property)
-                else -> null
+        addTypeConverter(object : FieldHandler {
+            override val execution = FieldHandlerFactory.createUpdateExecution<Any> { instance, property ->
+                when (instance) {
+                    is AnyCol -> updateAnyColVariable(instance, property, codeGen)
+                    is ColumnGroup<*> -> updateColumnGroupVariable(instance, property, codeGen)
+                    is AnyRow -> updateAnyRowVariable(instance, property, codeGen)
+                    is AnyFrame -> updateAnyFrameVariable(instance, property, codeGen)
+                    is ImportDataSchema -> updateImportDataSchemaVariable(instance, property)
+                    else -> error("${instance::class} should not be handled by Dataframe field handler")
+                }
             }
-        }
+            override fun accepts(value: Any?, property: KProperty<*>): Boolean {
+                return value is AnyCol ||
+                    value is ColumnGroup<*> ||
+                    value is AnyRow ||
+                    value is AnyFrame ||
+                    value is ImportDataSchema
+            }
+        })
 
         fun KotlinKernelHost.addDataSchemas(classes: List<KClass<*>>) {
             val code = classes.joinToString("\n") {

diff --git a/...ed-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/JupyterCodegenTests.kt b/...ed-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/JupyterCodegenTests.kt
@@ -301,6 +301,23 @@ class JupyterCodegenTests : JupyterReplTestCase() {
         res2.shouldBeInstanceOf<Unit>()
     }
 
+    @Test
+    fun `type converter does not conflict with other type converters`() {
+        @Language("kts")
+        val anotherTypeConverter = """
+            notebook.fieldsHandlersProcessor.register(
+                FieldHandlerFactory.createUpdateHandler<ByteArray>(TypeDetection.RUNTIME) { _, prop ->
+                     execute(prop.name + ".toList()").name
+                },
+                ProcessingPriority.LOW
+            )
+        """.trimIndent()
+        execEx(anotherTypeConverter)
+        execEx("val x = ByteArray(1)")
+        val res1 = execRaw("x")
+        res1.shouldBeInstanceOf<List<*>>()
+    }
+
     @Test
     fun `generate a new marker when dataframe marker is not a data schema so that columns are accessible with extensions`() {
         @Language("kts")

diff --git a/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/Integration.kt b/core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/Integration.kt
@@ -45,15 +45,8 @@ import org.jetbrains.kotlinx.dataframe.impl.renderType
 import org.jetbrains.kotlinx.dataframe.io.DataFrameHtmlData
 import org.jetbrains.kotlinx.dataframe.io.SupportedCodeGenerationFormat
 import org.jetbrains.kotlinx.dataframe.io.supportedFormats
-import org.jetbrains.kotlinx.jupyter.api.HTML
-import org.jetbrains.kotlinx.jupyter.api.JupyterClientType
-import org.jetbrains.kotlinx.jupyter.api.KotlinKernelHost
-import org.jetbrains.kotlinx.jupyter.api.Notebook
-import org.jetbrains.kotlinx.jupyter.api.VariableName
-import org.jetbrains.kotlinx.jupyter.api.declare
-import org.jetbrains.kotlinx.jupyter.api.libraries.ColorScheme
-import org.jetbrains.kotlinx.jupyter.api.libraries.JupyterIntegration
-import org.jetbrains.kotlinx.jupyter.api.libraries.resources
+import org.jetbrains.kotlinx.jupyter.api.*
+import org.jetbrains.kotlinx.jupyter.api.libraries.*
 import kotlin.reflect.KClass
 import kotlin.reflect.KProperty
 import kotlin.reflect.KType
@@ -281,16 +274,25 @@ internal class Integration(
         import("org.jetbrains.kotlinx.dataframe.dataTypes.*")
         import("org.jetbrains.kotlinx.dataframe.impl.codeGen.urlCodeGenReader")
 
-        updateVariable<Any> { instance, property ->
-            when (instance) {
-                is AnyCol -> updateAnyColVariable(instance, property, codeGen)
-                is ColumnGroup<*> -> updateColumnGroupVariable(instance, property, codeGen)
-                is AnyRow -> updateAnyRowVariable(instance, property, codeGen)
-                is AnyFrame -> updateAnyFrameVariable(instance, property, codeGen)
-                is ImportDataSchema -> updateImportDataSchemaVariable(instance, property)
-                else -> null
+        addTypeConverter(object : FieldHandler {
+            override val execution = FieldHandlerFactory.createUpdateExecution<Any> { instance, property ->
+                when (instance) {
+                    is AnyCol -> updateAnyColVariable(instance, property, codeGen)
+                    is ColumnGroup<*> -> updateColumnGroupVariable(instance, property, codeGen)
+                    is AnyRow -> updateAnyRowVariable(instance, property, codeGen)
+                    is AnyFrame -> updateAnyFrameVariable(instance, property, codeGen)
+                    is ImportDataSchema -> updateImportDataSchemaVariable(instance, property)
+                    else -> error("${instance::class} should not be handled by Dataframe field handler")
+                }
             }
-        }
+            override fun accepts(value: Any?, property: KProperty<*>): Boolean {
+                return value is AnyCol ||
+                    value is ColumnGroup<*> ||
+                    value is AnyRow ||
+                    value is AnyFrame ||
+                    value is ImportDataSchema
+            }
+        })
 
         fun KotlinKernelHost.addDataSchemas(classes: List<KClass<*>>) {
             val code = classes.joinToString("\n") {

diff --git a/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/JupyterCodegenTests.kt b/core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/JupyterCodegenTests.kt
@@ -301,6 +301,23 @@ class JupyterCodegenTests : JupyterReplTestCase() {
         res2.shouldBeInstanceOf<Unit>()
     }
 
+    @Test
+    fun `type converter does not conflict with other type converters`() {
+        @Language("kts")
+        val anotherTypeConverter = """
+            notebook.fieldsHandlersProcessor.register(
+                FieldHandlerFactory.createUpdateHandler<ByteArray>(TypeDetection.RUNTIME) { _, prop ->
+                     execute(prop.name + ".toList()").name
+                },
+                ProcessingPriority.LOW
+            )
+        """.trimIndent()
+        execEx(anotherTypeConverter)
+        execEx("val x = ByteArray(1)")
+        val res1 = execRaw("x")
+        res1.shouldBeInstanceOf<List<*>>()
+    }
+
     @Test
     fun `generate a new marker when dataframe marker is not a data schema so that columns are accessible with extensions`() {
         @Language("kts")