fix camelCase #1072

AndreiKingsley · 2025-02-21T12:33:02Z

fixes renameToCamelCase works is not as expected #988
Add kdocs and tests, also fixed code from Add guide for custom SQL database support with HSQLDB #986 (comment)_.

zaleslaw · 2025-02-24T12:27:19Z

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/impl/ToCamelCase.kt

+import io.kotest.matchers.shouldBe
+import org.junit.Test
+
+class ToCamelCase {


Nice tests!

zaleslaw

Great! Thanks for fixing this and docs update

Jolanrensen · 2025-02-24T12:07:14Z

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

- * This function renames all columns to `camelCase` by replacing all [delimiters][DELIMITERS_REGEX]
- * and converting the first char to lowercase.
- * Even [DataFrames][DataFrame] inside [FrameColumns][FrameColumn] are traversed recursively.
+ * This function renames all columns in this [DataFrame] to `camelCase` format.


*the camelCase format

Jolanrensen · 2025-02-24T12:08:30Z

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

+ * This function renames all columns in this [DataFrame] to `camelCase` format.
+ *
+ * All delimiters between words are removed, words are capitalized except for the first one.
+ * Places underscore between numbers.


Could you rewrite these two sentences? They don't flow well, and you jump from passive to active.

Jolanrensen · 2025-02-24T12:09:37Z

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

+ *
+ * All delimiters between words are removed, words are capitalized except for the first one.
+ * Places underscore between numbers.
+ * If the string does not contain any letters or numbers, it remains unchanged.


This no longer holds, does it?

letters or numbers

1.0.2 -> 1_0_2

._-. -> ._-.

oh you're right

Jolanrensen · 2025-02-24T12:11:26Z

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

+ *
+ * @return trhe formatted string in lowerCamelCase.
+ */
+public fun String.toCamelCaseByDelimiters(delimiters: Regex = CAMEL_DEFAULT_DELIMITERS_REGEX): String =


this pollutes the public String scope. Maybe keep it internal for now?

This is used in another module.

I can use friend modules

No it's fine for now. We will consider friend modules later probably, when we find out a safe way to use them.

Jolanrensen · 2025-02-24T12:11:57Z

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

-private const val DELIMITERS = "[_\\s]"
-public val DELIMITERS_REGEX: Regex = DELIMITERS.toRegex()
-public val DELIMITED_STRING_REGEX: Regex = ".+$DELIMITERS.+".toRegex()
+// Single regex to split words by non-alphanumeric characters, camelCase, and numbers



Jolanrensen · 2025-02-24T12:25:11Z

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

+ * "10-20-aa" -> "10_20Aa"
+ * ```
+ *
+ * @return trhe formatted string in lowerCamelCase.


Jolanrensen · 2025-02-24T12:25:42Z

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

+        split(delimiters)
+            .filter { it.isNotBlank() }
+            .map { it.lowercase() }
+            .joinNumbers("_")


maybe we can make this an argument of the function, could be useful to change later if we need to :)

Jolanrensen · 2025-02-24T12:51:06Z

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

+/**
+ * Joins consecutive numbers in a list with the given [separator].
+ */
+private fun List<String>.joinNumbers(separator: CharSequence): List<String> {


Hmm this seems a bit overcomplicated. It's not really clear what this function does and it's hard to check due to its complexity.
(What I think it does is [a1, 123, 345, bcd] -> [a1, 123_345, bcd], is that correct? This is then joined to "a1123_345Bcd". Actually, I think a1 and 123 should remain split as well.)

Wouldn't it be a lot simpler to perform a map-with-next and let joinWithCamelCaseString handle the joining part? Something like:

list.mapIndexed { i, current -> val next = list.getOrNull(i + 1) ?: return@mapIndexed current if (current.last().isDigit() && next.first().isDigit()) { current + "_" } else { current } }

This would do [a1, 123, 345, bcd] -> [a1_, 123_, 345, bcd], after which they can be joined together like "a1_123_345Bcd"

It assumes that all numbers and strings are separated, I'll write a comment.

aah that was unclear indeed. Still, it might do with some simplification :)

I'd prefer to separate numbers join logic

Jolanrensen · 2025-02-24T12:53:41Z

core/src/test/kotlin/org/jetbrains/kotlinx/dataframe/impl/ToCamelCase.kt

+            "singleword",
+            "word_with_underscores_and-dashes",
+            "10-20-aa",
+            "ROOM_1.11",


Could you also add some special characters, like æ or letters from other alphabets?

Jolanrensen · 2025-02-24T12:54:29Z

dataframe-openapi-generator/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/utils.kt

@@ -18,7 +17,7 @@ internal fun String.withoutTopInterfaceName(topInterfaceName: ValidFieldName): S
        this
    }

-internal fun String.snakeToLowerCamelCase(): String = toCamelCaseByDelimiters(DELIMITERS_REGEX)


Let's just deprecate this function or remove it

I'm hesitating because it's used in the implementation, I don't understand why and how it should work so I'm just afraid of breaking something

AndreiKingsley added 2 commits February 21, 2025 16:22

fix camelCase

c5c7f0a

update kdocs

cd4c7d8

AndreiKingsley requested review from Jolanrensen and zaleslaw February 21, 2025 12:33

zaleslaw reviewed Feb 24, 2025

View reviewed changes

zaleslaw approved these changes Feb 24, 2025

View reviewed changes

Jolanrensen requested changes Feb 24, 2025

View reviewed changes

camelCase fixes and special chars support

ed23939

AndreiKingsley requested a review from Jolanrensen February 24, 2025 16:14

Jolanrensen approved these changes Feb 24, 2025

View reviewed changes

AndreiKingsley merged commit 0aaee79 into master Feb 25, 2025
6 checks passed

AndreiKingsley deleted the camel_case_fix branch February 25, 2025 06:38

fix camelCase #1072

fix camelCase #1072

Uh oh!

Conversation

AndreiKingsley commented Feb 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaleslaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndreiKingsley Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AndreiKingsley Feb 24, 2025 •

edited

Loading