Skip to content

Commit fca4b8f

Browse files
committed
simplified sorting by constructor
1 parent 07d933b commit fca4b8f

File tree

6 files changed

+40
-90
lines changed

6 files changed

+40
-90
lines changed

core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType
2121
import org.jetbrains.kotlinx.dataframe.impl.isArray
2222
import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
2323
import org.jetbrains.kotlinx.dataframe.impl.projectUpTo
24-
import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors
24+
import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor
2525
import java.lang.reflect.InvocationTargetException
2626
import java.lang.reflect.Method
2727
import java.time.temporal.Temporal
@@ -178,8 +178,8 @@ internal fun convertToDataFrame(
178178
.filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
179179
}
180180

181-
// sort properties by order in constructors
182-
.sortWithConstructors(clazz)
181+
// sort properties by order in constructor
182+
.sortWithConstructor(clazz)
183183

184184
val columns = properties.mapNotNull {
185185
val property = it

core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame =
144144
internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame =
145145
MarkersExtractor.get(clazz).schema.createEmptyDataFrame()
146146

147+
/**
148+
* Returns a map of property names to their order in the primary/single constructor, if it exists,
149+
* `null` otherwise.
150+
*/
147151
internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map<String, Int>? =
148-
clazz.primaryConstructor
152+
(clazz.primaryConstructor ?: clazz.constructors.singleOrNull())
149153
?.parameters
150154
?.mapNotNull { it.name }
151155
?.mapIndexed { i, v -> v to i }
152156
?.toMap()
153157

154-
internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List<Map<String, Int>> =
155-
clazz.constructors
156-
.map { constructor ->
157-
constructor.parameters
158-
.mapNotNull { it.name }
159-
.mapIndexed { i, v -> v to i }
160-
.toMap()
161-
}
162-
163158
/**
164-
* Sorts [this] according to the order of them in the constructors of [klass].
165-
* It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting.
166-
* Finally, it falls back to lexicographical sorting if a property does not exist in any constructor.
159+
* Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass]
160+
* if it exists, else, it falls back to lexicographical sorting.
167161
*/
168-
internal fun <T> Iterable<KCallable<T>>.sortWithConstructors(klass: KClass<*>): List<KCallable<T>> {
162+
internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): List<KCallable<T>> {
169163
require(all { it.isGetterLike() })
170164
val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass)
171-
val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass)
172165

173-
// starting off lexicographically, sort properties according to the order of all constructors
174-
val allConstructorsSortedProperties = allConstructorsOrders
175-
.fold(this.sortedBy { it.columnName }) { props, constructorOrder ->
176-
props
177-
.withIndex()
178-
.sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i }
179-
.map { it.value }
180-
}.toList()
166+
// starting off lexicographically
167+
val lexicographicalColumns = sortedBy { it.columnName }
181168

169+
// if no primary constructor, return lexicographical order
182170
if (primaryConstructorOrder == null) {
183-
return allConstructorsSortedProperties
171+
return lexicographicalColumns
184172
}
185173

186-
// prefer to sort properties according to the order in the primary constructor if it exists.
187-
// if a property does not exist in the primary constructor, fall back to the other order
188-
174+
// else sort the ones in the primary constructor first according to the order in there
175+
// leave the rest at the end in lexicographical order
189176
val (propsInConstructor, propsNotInConstructor) =
190-
this.partition { it.columnName in primaryConstructorOrder.keys }
191-
192-
val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName }
177+
lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
193178

194-
return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } +
195-
propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) }
179+
return propsInConstructor
180+
.sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
196181
}

core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -319,21 +319,11 @@ class CreateDataFrameTests {
319319
private var a: Int = 0
320320
private var b: String = ""
321321

322-
constructor()
323-
324322
constructor(b: String, a: Int) {
325323
this.a = a
326324
this.b = b
327325
}
328326

329-
constructor(b: String) {
330-
this.b = b
331-
}
332-
333-
constructor(a: Int) {
334-
this.a = a
335-
}
336-
337327
fun getA(): Int = a
338328
fun setA(a: Int) {
339329
this.a = a

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/api/toDataFrame.kt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import org.jetbrains.kotlinx.dataframe.impl.getListType
2121
import org.jetbrains.kotlinx.dataframe.impl.isArray
2222
import org.jetbrains.kotlinx.dataframe.impl.isGetterLike
2323
import org.jetbrains.kotlinx.dataframe.impl.projectUpTo
24-
import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructors
24+
import org.jetbrains.kotlinx.dataframe.impl.schema.sortWithConstructor
2525
import java.lang.reflect.InvocationTargetException
2626
import java.lang.reflect.Method
2727
import java.time.temporal.Temporal
@@ -178,8 +178,8 @@ internal fun convertToDataFrame(
178178
.filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
179179
}
180180

181-
// sort properties by order in constructors
182-
.sortWithConstructors(clazz)
181+
// sort properties by order in constructor
182+
.sortWithConstructor(clazz)
183183

184184
val columns = properties.mapNotNull {
185185
val property = it

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/impl/schema/Utils.kt

Lines changed: 17 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -144,53 +144,38 @@ internal fun DataFrameSchema.createEmptyDataFrame(numberOfRows: Int): AnyFrame =
144144
internal fun createEmptyDataFrameOf(clazz: KClass<*>): AnyFrame =
145145
MarkersExtractor.get(clazz).schema.createEmptyDataFrame()
146146

147+
/**
148+
* Returns a map of property names to their order in the primary/single constructor, if it exists,
149+
* `null` otherwise.
150+
*/
147151
internal fun getPropertyOrderFromPrimaryConstructor(clazz: KClass<*>): Map<String, Int>? =
148-
clazz.primaryConstructor
152+
(clazz.primaryConstructor ?: clazz.constructors.singleOrNull())
149153
?.parameters
150154
?.mapNotNull { it.name }
151155
?.mapIndexed { i, v -> v to i }
152156
?.toMap()
153157

154-
internal fun getPropertyOrderFromAllConstructors(clazz: KClass<*>): List<Map<String, Int>> =
155-
clazz.constructors
156-
.map { constructor ->
157-
constructor.parameters
158-
.mapNotNull { it.name }
159-
.mapIndexed { i, v -> v to i }
160-
.toMap()
161-
}
162-
163158
/**
164-
* Sorts [this] according to the order of them in the constructors of [klass].
165-
* It prefers the primary constructor if it exists, else it falls back to the other constructors to do the sorting.
166-
* Finally, it falls back to lexicographical sorting if a property does not exist in any constructor.
159+
* Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass]
160+
* if it exists, else, it falls back to lexicographical sorting.
167161
*/
168-
internal fun <T> Iterable<KCallable<T>>.sortWithConstructors(klass: KClass<*>): List<KCallable<T>> {
162+
internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): List<KCallable<T>> {
169163
require(all { it.isGetterLike() })
170164
val primaryConstructorOrder = getPropertyOrderFromPrimaryConstructor(klass)
171-
val allConstructorsOrders = getPropertyOrderFromAllConstructors(klass)
172165

173-
// starting off lexicographically, sort properties according to the order of all constructors
174-
val allConstructorsSortedProperties = allConstructorsOrders
175-
.fold(this.sortedBy { it.columnName }) { props, constructorOrder ->
176-
props
177-
.withIndex()
178-
.sortedBy { (i, it) -> constructorOrder[it.columnName] ?: i }
179-
.map { it.value }
180-
}.toList()
166+
// starting off lexicographically
167+
val lexicographicalColumns = sortedBy { it.columnName }
181168

169+
// if no primary constructor, return lexicographical order
182170
if (primaryConstructorOrder == null) {
183-
return allConstructorsSortedProperties
171+
return lexicographicalColumns
184172
}
185173

186-
// prefer to sort properties according to the order in the primary constructor if it exists.
187-
// if a property does not exist in the primary constructor, fall back to the other order
188-
174+
// else sort the ones in the primary constructor first according to the order in there
175+
// leave the rest at the end in lexicographical order
189176
val (propsInConstructor, propsNotInConstructor) =
190-
this.partition { it.columnName in primaryConstructorOrder.keys }
191-
192-
val allConstructorsSortedPropertyNames = allConstructorsSortedProperties.map { it.columnName }
177+
lexicographicalColumns.partition { it.columnName in primaryConstructorOrder.keys }
193178

194-
return propsInConstructor.sortedBy { primaryConstructorOrder[it.columnName] } +
195-
propsNotInConstructor.sortedBy { allConstructorsSortedPropertyNames.indexOf(it.columnName) }
179+
return propsInConstructor
180+
.sortedBy { primaryConstructorOrder[it.columnName] } + propsNotInConstructor
196181
}

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/api/toDataFrame.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -319,21 +319,11 @@ class CreateDataFrameTests {
319319
private var a: Int = 0
320320
private var b: String = ""
321321

322-
constructor()
323-
324322
constructor(b: String, a: Int) {
325323
this.a = a
326324
this.b = b
327325
}
328326

329-
constructor(b: String) {
330-
this.b = b
331-
}
332-
333-
constructor(a: Int) {
334-
this.a = a
335-
}
336-
337327
fun getA(): Int = a
338328
fun setA(a: Int) {
339329
this.a = a

0 commit comments

Comments
 (0)