Missing LocalTime converters

[Jolanrensen]

As discovered by #887 (comment), we're missing some converters from kotlinx/java LocalTime, making auto conversion from kotlinx->java LocalTime not possible.

(auto)-converters are defined here

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

Line 250 in a4104d7

else -> when (fromClass) {

• We need to check whether all possible conversions are present. Especially that kotlinx <-> java conversions are all there.
• java/kotlinx LocalTime as from classes are missing and should be added

The website also doesn't seem to cover all our converters: https://kotlin.github.io/dataframe/convert.html so it should probably be updated

[Ahar28]

Hi @Jolanrensen ! I'm interested in contributing to this issue as a new contributor.

Proposed Solution

I've updated the createConverter function to add converters for LocalTime:

import kotlinx.datetime.toKotlinLocalTime

LocalTime::class -> when (toClass) {
    JavaLocalTime::class -> convert<LocalTime> { it.toJavaLocalTime() }
    else -> null
}

JavaLocalTime::class -> when (toClass) {
    LocalTime::class -> convert<JavaLocalTime> { it.toKotlinLocalTime() }
    else -> null
}

Could you please let me know if this approach aligns with the requirements or if you have any feedback?

[Jolanrensen]

Hi @Ahar28, thanks for offering your help :)

I was thinking that we might be missing some more conversions like from/to Instant or LocalDateTime, but on second thought, I wouldn't know what a "default date" would need to be. (Like year 0? 1970?), so it might be best to leave those conversions out. Same with conversions to Long/Int (like, would it be the milliseconds or seconds of the current day? No obvious default exists).

So, java <-> kotlin conversion would be enough I suppose. However, do check the other conversions if you're willing to make a PR. I see for instance that (kotlin) LocalTime is missing from the Long conversions while JavaLocalTime is present.

We'd also need some tests of course, and finally a small update to the website (in the docs/StarDustDocs/topics folder) with the newly supported types. Would you be up for that? :)

[Ahar28]

Hi @Jolanrensen, yes I'm up for it !

I'll make the changes and get it reviewed from you. Thank you for giving me this opportunity.

[Jolanrensen]

@Ahar28 did you have a chance to look at it already? We would like to have this bug fixed for 0.15.0 ideally :)

[Ahar28]

Hi @Jolanrensen , sorry for the delay on my part. But I'm currently tied up with some other commitments and won’t be able to take this on until next week. If the timeline allows, I’ll look into it as soon as I’m available.