Skip to content

Commit 3daf3e3

Browse files
committed
Replace isComparable check with the correct one
Refactored `isComparable` check to use the `isComparable` extension function directly on the column. Added test case for sorting columns containing mixed data types of strings and integers to ensure correct functionality. Removed redundant KType extension method `isComparable`.
1 parent 12deade commit 3daf3e3

File tree

4 files changed

+50
-20
lines changed

4 files changed

+50
-20
lines changed

core/generated-sources/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/KotlinNotebookPluginUtils.kt

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,13 @@ import org.jetbrains.kotlinx.dataframe.api.at
2727
import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
2828
import org.jetbrains.kotlinx.dataframe.api.frames
2929
import org.jetbrains.kotlinx.dataframe.api.getColumn
30-
import org.jetbrains.kotlinx.dataframe.api.getValueOrNull
3130
import org.jetbrains.kotlinx.dataframe.api.into
31+
import org.jetbrains.kotlinx.dataframe.api.isComparable
3232
import org.jetbrains.kotlinx.dataframe.api.sortWith
3333
import org.jetbrains.kotlinx.dataframe.api.toDataFrame
3434
import org.jetbrains.kotlinx.dataframe.api.values
3535
import org.jetbrains.kotlinx.dataframe.columns.ColumnPath
3636
import org.jetbrains.kotlinx.dataframe.impl.ColumnNameGenerator
37-
import org.jetbrains.kotlinx.dataframe.type
38-
import kotlin.collections.sortWith
39-
import kotlin.reflect.KType
40-
import kotlin.reflect.full.isSubtypeOf
41-
import kotlin.reflect.typeOf
42-
import kotlin.toString
4337

4438
/**
4539
* A class with utility methods for Kotlin Notebook Plugin integration.
@@ -113,7 +107,7 @@ public object KotlinNotebookPluginUtils {
113107
private fun createComparator(sortKeys: List<ColumnPath>, isDesc: List<Boolean>): Comparator<DataRow<*>> {
114108
return Comparator { row1, row2 ->
115109
for ((key, desc) in sortKeys.zip(isDesc)) {
116-
val comparisonResult = if (row1.df().getColumn(key).type.isComparable()) {
110+
val comparisonResult = if (row1.df().getColumn(key).isComparable()) {
117111
compareComparableValues(row1, row2, key, desc)
118112
} else {
119113
compareStringValues(row1, row2, key, desc)
@@ -161,8 +155,6 @@ public object KotlinNotebookPluginUtils {
161155
}
162156
}
163157

164-
private fun KType.isComparable(): Boolean = this.isSubtypeOf(typeOf<Comparable<*>>())
165-
166158
internal fun isDataframeConvertable(dataframeLike: Any?): Boolean =
167159
when (dataframeLike) {
168160
is Pivot<*>,

core/generated-sources/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/RenderingTests.kt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,29 @@ class RenderingTests : JupyterReplTestCase() {
583583
bFields shouldBe listOf(5, 4, 3, 2, 1)
584584
}
585585

586+
@Test
587+
fun `test sortByColumns for column that contains string and int`() {
588+
val json = executeScriptAndParseDataframeResult(
589+
"""
590+
val df = dataFrameOf("mixed")(
591+
5,
592+
"10",
593+
2,
594+
"4",
595+
"1"
596+
)
597+
val res = KotlinNotebookPluginUtils.sortByColumns(df, listOf(listOf("mixed")), listOf(true))
598+
KotlinNotebookPluginUtils.convertToDataFrame(res)
599+
""".trimIndent(),
600+
)
601+
602+
json.extractColumn<String>(0, "mixed") shouldBe "5"
603+
json.extractColumn<String>(1, "mixed") shouldBe "4"
604+
json.extractColumn<String>(2, "mixed") shouldBe "2"
605+
json.extractColumn<String>(3, "mixed") shouldBe "10"
606+
json.extractColumn<String>(4, "mixed") shouldBe "1"
607+
}
608+
586609
companion object {
587610
/**
588611
* Set the system property for the IDE version needed for specific serialization testing purposes.

core/src/main/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/KotlinNotebookPluginUtils.kt

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,13 @@ import org.jetbrains.kotlinx.dataframe.api.at
2727
import org.jetbrains.kotlinx.dataframe.api.dataFrameOf
2828
import org.jetbrains.kotlinx.dataframe.api.frames
2929
import org.jetbrains.kotlinx.dataframe.api.getColumn
30-
import org.jetbrains.kotlinx.dataframe.api.getValueOrNull
3130
import org.jetbrains.kotlinx.dataframe.api.into
31+
import org.jetbrains.kotlinx.dataframe.api.isComparable
3232
import org.jetbrains.kotlinx.dataframe.api.sortWith
3333
import org.jetbrains.kotlinx.dataframe.api.toDataFrame
3434
import org.jetbrains.kotlinx.dataframe.api.values
3535
import org.jetbrains.kotlinx.dataframe.columns.ColumnPath
3636
import org.jetbrains.kotlinx.dataframe.impl.ColumnNameGenerator
37-
import org.jetbrains.kotlinx.dataframe.type
38-
import kotlin.collections.sortWith
39-
import kotlin.reflect.KType
40-
import kotlin.reflect.full.isSubtypeOf
41-
import kotlin.reflect.typeOf
42-
import kotlin.toString
4337

4438
/**
4539
* A class with utility methods for Kotlin Notebook Plugin integration.
@@ -113,7 +107,7 @@ public object KotlinNotebookPluginUtils {
113107
private fun createComparator(sortKeys: List<ColumnPath>, isDesc: List<Boolean>): Comparator<DataRow<*>> {
114108
return Comparator { row1, row2 ->
115109
for ((key, desc) in sortKeys.zip(isDesc)) {
116-
val comparisonResult = if (row1.df().getColumn(key).type.isComparable()) {
110+
val comparisonResult = if (row1.df().getColumn(key).isComparable()) {
117111
compareComparableValues(row1, row2, key, desc)
118112
} else {
119113
compareStringValues(row1, row2, key, desc)
@@ -161,8 +155,6 @@ public object KotlinNotebookPluginUtils {
161155
}
162156
}
163157

164-
private fun KType.isComparable(): Boolean = this.isSubtypeOf(typeOf<Comparable<*>>())
165-
166158
internal fun isDataframeConvertable(dataframeLike: Any?): Boolean =
167159
when (dataframeLike) {
168160
is Pivot<*>,

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/jupyter/RenderingTests.kt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,29 @@ class RenderingTests : JupyterReplTestCase() {
583583
bFields shouldBe listOf(5, 4, 3, 2, 1)
584584
}
585585

586+
@Test
587+
fun `test sortByColumns for column that contains string and int`() {
588+
val json = executeScriptAndParseDataframeResult(
589+
"""
590+
val df = dataFrameOf("mixed")(
591+
5,
592+
"10",
593+
2,
594+
"4",
595+
"1"
596+
)
597+
val res = KotlinNotebookPluginUtils.sortByColumns(df, listOf(listOf("mixed")), listOf(true))
598+
KotlinNotebookPluginUtils.convertToDataFrame(res)
599+
""".trimIndent(),
600+
)
601+
602+
json.extractColumn<String>(0, "mixed") shouldBe "5"
603+
json.extractColumn<String>(1, "mixed") shouldBe "4"
604+
json.extractColumn<String>(2, "mixed") shouldBe "2"
605+
json.extractColumn<String>(3, "mixed") shouldBe "10"
606+
json.extractColumn<String>(4, "mixed") shouldBe "1"
607+
}
608+
586609
companion object {
587610
/**
588611
* Set the system property for the IDE version needed for specific serialization testing purposes.

0 commit comments

Comments
 (0)