Builtin Date_Time, Time_Of_Day, Zone #3658

hubertp · 2022-08-18T14:40:43Z

Pull Request Description

Improved polyglot support for Date_Time (formerly Time), Time_Of_Day and
Zone. This follows the pattern introduced for Enso Date.

Related to https://www.pivotaltracker.com/story/show/182973664 and https://www.pivotaltracker.com/n/projects/2539304/stories/182987210

Important Notes

Minor caveat - in tests for Date, had to bend a lot for JS Date to pass.
This is because JS Date is not really only a Date, but also a Time and
Timezone, previously we just didn't consider the latter.
Also, JS Date does not deal well with setting timezones so the trick I
used is to first call foreign function returning a polyglot JS Date,
which is converted to ZonedDateTime and only then set the correct
timezone. That way none of the existing tests had to be changed or
special cased.

Additionally, JS deals with milliseconds rather than nanoseconds so
there is loss in precision, as noted in Time_Spec.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoZone.java

test/Table_Tests/src/Data_Formatter_Spec.enso

std-bits/table/src/main/java/org/enso/table/formatting/TimeFormatter.java

std-bits/table/src/main/java/org/enso/table/data/table/Column.java

engine/runtime/src/main/java/org/enso/interpreter/runtime/type/Types.java

...ne/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Zone.enso

JaroslavTulach

Looks great to me overall. Some observations inline.

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Time_Of_Day.enso

radeusgd · 2022-08-22T15:08:29Z

distribution/lib/Standard/Table/0.0.0-dev/src/Data/Data_Formatter.enso

-        Integer -> self.make_integer_parser
-        Decimal -> self.make_decimal_parser
-        Boolean -> self.make_boolean_parser
-        _ ->
-            if datatype == Date then self.make_date_parser else
-                if datatype == Time then self.make_datetime_parser else
-                    if datatype == Time_Of_Day then self.make_time_parser else
-                        Error.throw (Illegal_Argument_Error "Unsupported datatype: "+datatype.to_text)
+        Integer     -> self.make_integer_parser
+        Decimal     -> self.make_decimal_parser
+        Boolean     -> self.make_boolean_parser
+        Date        -> self.make_date_parser
+        Date_Time   -> self.make_datetime_parser
+        Time_Of_Day -> self.make_time_parser
+        _           -> Error.throw (Illegal_Argument_Error "Unsupported datatype: "+datatype.to_text)



engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoDateTime.java

std-bits/base/src/main/java/org/enso/base/Time_Utils.java

test/Table_Tests/src/Table_Time_Of_Day_Spec.enso

test/Table_Tests/src/Table_Time_Spec.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time.enso

std-bits/table/src/main/java/org/enso/table/formatting/TimeFormatter.java

test/Table_Tests/src/Table_Time_Spec.enso

test/Table_Tests/src/Data_Formatter_Spec.enso

Improved polyglot support for Date_Time (formerly Time), Time_Of_Day and Zone. This follows the pattern introduced for Enso Date. Minor caveat - in tests for Date, had to bend a lot for JS Date to pass. This is because JS Date is not really only a Date, but also a Time and Timezone, previously we just didn't consider the latter. Also, JS Date does not deal well with setting timezones so the trick I used is to first call foreign function returning a polyglot JS Date, which is converted to ZonedDateTime and only then set the correct timezone. That way none of the existing tests had to be changes or special cased. Additionally, JS deals with milliseconds rather than nanoseconds so there is loss in precision, as noted in Time_Spec.

Column reader didn't take into account timezone but that was a mistake since then it wouldn't map to Enso's Date_Time. Added tests that check it now.

Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>

hubertp · 2022-08-23T15:57:52Z

@radeusgd I think I addressed all your comments

...ne/runtime/src/main/java/org/enso/interpreter/node/callable/resolver/HostMethodCallNode.java

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoDateTime.java

JaroslavTulach · 2022-08-24T05:28:37Z

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoDateTime.java

+  }
+
+  @Builtin.Method(description = "Return the number of seconds from the Unix epoch.")
+  public long toEpochSeconds() {


My recent experience with @TruffleBoundary suggests that many of these methods shall be behind the boundary.

I can add some, but at the moment without any gatekeepers it feels like a slightly random process.

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/EnsoDateTime.java

jdunkerley

Thanks for this @hubertp - this gives us a fantastic base to build on to build out the functions we need.