Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Kotlin Datetime Optional #892

Closed
hantsy opened this issue Sep 27, 2024 · 3 comments
Closed

Make Kotlin Datetime Optional #892

hantsy opened this issue Sep 27, 2024 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@hantsy
Copy link

hantsy commented Sep 27, 2024

Given an example like this.

data class (
   @ColumnName("Start")
   val startedTime:LocalTime // java 8 DateTime
)

Which works well in the previous version.

When upgrading to 0.14, I got the exception like:

Type converter from kotlinx.datetime.LocalTime to java.time.LocalTime? is not found for column

Now I have to add a manual convert to make it work.

   dataFrame
   .convert{ "Start"<kotlinx.datetime.LocalTime>()}.with{
        it.toJavaLocalTime()
   }
   

Even though we are using Kotlin in our backend, Java 8 Datetime is more popular than Kotlin DateTime.

We do not want to introduce more dependencies that we do not use.

Originally posted by @hantsy in #887 (comment)

It is better to support Java 8 Datetime by default, and make Kotlinx datetime optional for dataframe-core.

@zaleslaw zaleslaw added this to the Backlog milestone Sep 30, 2024
@zaleslaw zaleslaw added the enhancement New feature or request label Sep 30, 2024
@Jolanrensen
Copy link
Collaborator

Specific issue can be solved by fixing #894.

@hantsy
Copy link
Author

hantsy commented Oct 30, 2024

The conversion is resolved, but I hope to introduce fewer deps in projects.

@Jolanrensen
Copy link
Collaborator

@hantsy I think it makes sense for DataFrame to have kotlinx dependencies, as DataFrame itself is kotlinx after all. If DataFrame ever hopes to go multiplatform, we'll have to use kotlinx datetime and kotlinx serialization instead of java-specific libraries anyways. Of course, we'll still support java specific types, like java.time.*, but they won't be the default type. Similarly, if Kotlin ever introduces their own BigDecimal and BigInteger, we'll make those the default, instead of the Java ones.

At the same time, we indeed also strive to have fewer dependencies in the project, to shrink the core module. This we try to do by moving integration components to optional modules. It's a bit time-consuming though, as many components have become interwoven with each other. But, to give an example, we're in the progress of extracting CSV (and its dependencies), we split the OpenAPI module in two (OpenAPI is already optional, but now it's split into a runtime component and a gradle plugin dependency, shrinking the shipped jar further), and we were trying to extract the Jupyter/notebooks integration from the core too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants