Transformation into data class corrupts TimeZones of Instant

[martinitus]

Reproduction:

@Test
fun foo() {

    data class TimeZonesTest(val with_timezone_offset: Instant, val without_timezone_offset: LocalDateTime)

    val csvContent =
        """
        with_timezone_offset,without_timezone_offset
        2024-12-12T13:00:00+01:00,2024-12-12T13:00:00
        """.trimIndent()

    val df = DataFrame.readCsv(
        csvContent.byteInputStream(),
//            colTypes = mapOf("with_timezone_offset" to ColType.Instant)   // *1
//            parserOptions = ParserOptions(dateTimeFormatter = ISO_OFFSET_DATE_TIME), // *2
    )

    println(df)
    println(df.schema())
    val parsed = df.toListOf<TimeZonesTest>().first()
    assertEquals(Instant.parse("2024-12-12T13:00:00+01:00"), parsed.with_timezone_offset)
}

This outputs:

with_timezone_offset without_timezone_offset
0     2024-12-12T12:00        2024-12-12T13:00

with_timezone_offset: kotlinx.datetime.LocalDateTime
without_timezone_offset: kotlinx.datetime.LocalDateTime

org.opentest4j.AssertionFailedError: 
Expected :2024-12-12T12:00:00Z
Actual   :2024-12-12T11:00:00Z

Changing the dateTimeFormatter (*2) has no effect on the test outcome.
Explicitly telling the parser to parse as Instant (*1) fixes the issue.

However, following the principle of least surprise IMHO it would be A LOT better to:

• either have the conversion fail because the colum was processed as LocalDateTime, hence has no timezone information, and hence cannot be converted into an instant.
• or automagically detect, that there is a timezone in the data, and directly parse the column as Instant.

[martinitus]

I guess I could figure out how to change the implementation to fail (probably boils down to remove the respective converter somewhere). I am not sure if I could figure out how implement the "automagical instant detection" approach.

[Jolanrensen]

Hi @martinitus, thanks for letting us know!

I see you're using the new experimental DataFrame.readCsv() function, so I appreciate the feedback :)

Normally, DataFrame uses the parse operation in the order described in the docs, so Instant is checked before LocalDateTime. You can test this by running your input for a single column, like:

val column = columnOf("2024-12-12T13:00:00+01:00").parse()
println(column.type()) // Instant

However, for the new readCsv() implementation, we rely on Deephaven CSV which has its own type inference and an extremely fast date-time parser, which is enabled by default (for now). Deephaven parses 2024-12-12T13:00:00+01:00, and 2024-12-12T13:00:00 as LocalDateTime("2024-12-12T12:00"), and LocalDateTime("2024-12-12T13:00") respectively.

This looks correct to me; it's just that, since Deephaven cannot create Instants, it corrects the offset to create a LocalDateTime instead. This deletes the timezone information. If you then convert this LocalDateTime column to Instant with toListOf<TimeZonesTest>() you will thus get Instant("2024-12-12T12:00:00Z").

You can skip Deephaven's type inference for any column by providing a ColType.String, which allows you to call the .parse() operation manually afterwards, or you can supply any other type like ColType.Instant, like you did.

I do agree that it's confusing to have two different "defaults" for inference of types. The user shouldn't have to worry about which implementation is used to infer types and the results should be predictable. We already disable the Deephaven LocalDateTime parser whenever a user provides a non-US Locale, a dateTime pattern, or -formatter, but I suppose, we can disable it altogether unless a user explicitly provides ColType.LocalDateTime.