From 04c096d352a8cce563d9847ca571ddcea8845ffb Mon Sep 17 00:00:00 2001
From: "Anatoly.Nikitin" <Anatoly.Nikitin@jetbrains.com>
Date: Wed, 15 Dec 2021 02:01:39 +0300
Subject: [PATCH] Fix #32. Use order of primary constructor arguments to sort
 columns created in `toDataFrame()`.

---
 .../impl/api/{createDataFrame.kt => toDataFrame.kt}    |  5 ++++-
 .../api/{createDataFrame.kt => toDataFrame.kt}         | 10 ++++++++++
 .../dataframe/testSets/person/DataFrameTests.kt        |  7 +++----
 3 files changed, 17 insertions(+), 5 deletions(-)
 rename src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/{createDataFrame.kt => toDataFrame.kt} (96%)
 rename src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/{createDataFrame.kt => toDataFrame.kt} (85%)

diff --git a/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/createDataFrame.kt b/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
similarity index 96%
rename from src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/createDataFrame.kt
rename to src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
index d4a0799567..c63bcf1ed7 100644
--- a/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/createDataFrame.kt
+++ b/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt
@@ -21,6 +21,7 @@ import kotlin.reflect.KProperty
 import kotlin.reflect.KVisibility
 import kotlin.reflect.full.isSubclassOf
 import kotlin.reflect.full.memberProperties
+import kotlin.reflect.full.primaryConstructor
 import kotlin.reflect.full.withNullability
 import kotlin.reflect.jvm.javaField
 
@@ -91,10 +92,12 @@ internal fun convertToDataFrame(
     preserves: Set<KClass<*>>,
     depth: Int
 ): AnyFrame {
+    val constructorParameters = clazz.primaryConstructor?.parameters?.mapNotNull { it.name }?.mapIndexed { i, v -> v to i }?.toMap() ?: emptyMap()
+
     val properties = roots.ifEmpty {
         clazz.memberProperties
             .filter { it.visibility == KVisibility.PUBLIC && it.parameters.toList().size == 1 }
-    }
+    }.sortedBy { constructorParameters[it.name] ?: Int.MAX_VALUE }
 
     val columns = properties.mapNotNull {
         val property = it
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/createDataFrame.kt b/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
similarity index 85%
rename from src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/createDataFrame.kt
rename to src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
index a8ce179ac8..8e6afd1a6c 100644
--- a/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/createDataFrame.kt
+++ b/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt
@@ -54,4 +54,14 @@ class CreateDataFrameTests {
         res["d"].type() shouldBe typeOf<Int?>()
         res["e"].type() shouldBe typeOf<Int>()
     }
+
+    @Test
+    fun `preserve fields order`() {
+        class B(val x: Int, val c: String, d: Double) {
+            val b: Int = x
+            val a: Double = d
+        }
+
+        listOf(B(1, "a", 2.0)).toDataFrame().columnNames() shouldBe listOf("x", "c", "a", "b")
+    }
 }
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt
index db0c08a693..570c1c736a 100644
--- a/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt
+++ b/src/test/kotlin/org/jetbrains/kotlinx/dataframe/testSets/person/DataFrameTests.kt
@@ -2183,7 +2183,7 @@ class DataFrameTests : BaseTest() {
         list shouldBe df.convertTo<Target>().toList()
 
         val listDf = list.toDataFrame()
-        listDf shouldBe df.reorderColumnsByName()
+        listDf shouldBe df
         listDf.toList() shouldBe list
     }
 
@@ -2202,7 +2202,6 @@ class DataFrameTests : BaseTest() {
 
         val listDf = list.toDataFrame(depth = 2)
         listDf shouldBe grouped.update { getFrameColumn("students") }.with { it.remove("city") }
-            .reorderColumnsByName()
 
         listDf.toList() shouldBe list
     }
@@ -2221,7 +2220,7 @@ class DataFrameTests : BaseTest() {
         data class Info(val age: Int, val weight: Int?)
 
         @DataSchema
-        data class Target(val name: String, val city: String?, val info: Info)
+        data class Target(val name: String, val info: Info, val city: String?)
 
         val grouped = typed.group { age and weight }.into("info")
 
@@ -2229,7 +2228,7 @@ class DataFrameTests : BaseTest() {
         list shouldBe grouped.convertTo<Target>().toList()
 
         val listDf = list.toDataFrame(depth = 2)
-        listDf shouldBe grouped.reorderColumnsByName(dfs = true)
+        listDf shouldBe grouped
         listDf.toList() shouldBe list
     }