Support Java Record in toDataFrame conversion#1682
Conversation
| options.release.set(16) | ||
| } | ||
|
|
||
| tasks.named<KotlinCompile>("compileTestKotlin") { |
There was a problem hiding this comment.
This may be a slight issue, as this sets áll :core tests to java 16. We claim we support Java 8 for DataFrame, but we may not catch all issues correctly if we don't test with java 8. I found differences/bugs, for instance, with parsing certain Doubles in RTL locales which only occurred in Java 11, but not in 8 and the other way around.
There was a problem hiding this comment.
is there maybe a mechanism to only run java-record tests with jvm 16 while keeping the others at 8?
There was a problem hiding this comment.
This goes both way, isn't it? we claim to support java 21 but don't test against it. I suggest, if we're interested, to specifically run tests compiled with JDK8 compatibility using JDK8, JDK17, JDK 21. Start with :core. It can be done
| false | ||
| } | ||
|
|
||
| private val getRecordComponentsMethod: java.lang.reflect.Method? by lazy { |
There was a problem hiding this comment.
You can probably just import Method right? Or do you want to emphasize it's from Java? In that case I'd recommend importing it with the alias JavaMethod :) I do the same for Dates, Instants etc.
| ?.mapIndexed { i, v -> v to i } | ||
| ?.toMap() | ||
| if (clazz.isJavaRecord) { | ||
| // Can fail for Java records with primitive types due to KT-58649 |
There was a problem hiding this comment.
interesting... Does it always fail? Otherwise, we could maybe catch the exception so the order is correct if it doesn't fail or the issue is fixed on the reflection side.
There was a problem hiding this comment.
it always fails if there's a primitive type in the "constructor", like int. I'd wait until this is fixed on Kotlin side, logic would seem unstable + strange to reproduce it in the plugin (checking if there's a primitive..?)
| * Sorts [this] according to the order of their [columnName] in the primary/single constructor of [klass] | ||
| * if it exists, else, it falls back to lexicographical sorting. | ||
| */ | ||
| internal fun <T> Iterable<KCallable<T>>.sortWithConstructor(klass: KClass<*>): List<KCallable<T>> { |
There was a problem hiding this comment.
I assume you removed this check for records? Maybe instead of removing it, you could skip the check only when dealing with a record
|
Nice! |
2 participants
No description provided.