[Issue 24704] DateTime.fromISOExtString Does Not Support ISO8601 Time Unit Fractions (page 2)

August 16

[Issue 24704] DateTime.fromISOExtString Does Not Support ISO8601 Time Unit Fractions

Posted by Vijay Nayar

Permalink

Vijay Nayar

Permalink

https://issues.dlang.org/show_bug.cgi?id=24704

Vijay Nayar <madric@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|WONTFIX                     |---

--- Comment #9 from Vijay Nayar <madric@gmail.com> ---
I think the error message should be altered to prevent confusion in the future. Currently it states:

> Invalid ISO Extended String: 08:13:23.000

This message gives the impression that DateTime is supporting the full ISO Extended String standard.

One can also reproduce the error by having a fraction on the minute unit:

> Invalid ISO Extended String: 08:13.000

Regardless of whether DateTime is meant to use second or millisecond resolution, `fromISOExtString` claims to support ISO Ext Strings. The ISO standard doesn't change simply because of an implementation detail of DateTime.

Because the ISO standard does not control the number of decimal digits that the fraction can have, in fact, every implementation will not be able to fully represent a valid ISO string, but they continue to accept the ISO standard, extracting as much information as they can.

For example, a utility that only supports millisecond resolution would still be able to parse an ISO string of "2024-08-15T08:13:25.123456789", only it will only support saving a value equivalent to "2024-08-15T08:13:25.123".

In my opinion, there are still two issues remaining:
1. Valid ISO Strings within the capabilities of `DateTime` are being rejected,
e.g. "2024-08-15T08:13.5".
2. Valid ISO Strings beyond the resolution of `DateTime`, like
"2024-08-15T08:13:25.123", should either be parsed to include as much
information as they can, or the error message should be clarified rather than
claiming that the string is not ISO.

--

https://issues.dlang.org/show_bug.cgi?id=24704 --- Comment #10 from Vijay Nayar <madric@gmail.com> --- As a point of reference, the approach taken by Java's LocalTime (which has nanosecond precision) when given a string that has more data than it can handle, is to throw a parse exception. E.g. the following action produces the following error: ``` // 10 fraction digits, 1 more than is supported LocalTime myTime = LocalTime.parse("08:15:23.12345678910") ``` ``` Exception in thread "main" java.time.format.DateTimeParseException: Text '08:15:23.12345678910' could not be parsed, unparsed text found at index 18 ``` --

https://issues.dlang.org/show_bug.cgi?id=24704 Carsten Schlote <schlote@vahanus.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |schlote@vahanus.net --- Comment #11 from Carsten Schlote <schlote@vahanus.net> --- I can see good reasons to add a 'fraction of a second' to TimeOfDay. In this case it would be really easy to add the missing functionality to to/fromISOExtString. And it also fits reality a little bit better, because fractions of a seconds do matter in real life. However, there are also good reasons to avoid changes in the core libs without very good reasons. At least it will increase memory footprint because you must store the fraction of a second somewhere. So any change must be very intensively and well tested. Otherwise, if we add sucha fraction, in return we would get: - No/less info ist lost, when cast from SysTime to DateTime. And vice versa. - No more confusion with different variant of from ISOExtString. --

https://issues.dlang.org/show_bug.cgi?id=24704 --- Comment #12 from Jonathan M Davis <issues.dlang@jmdavisProg.com> --- The documentation on fromISOExtString on each of the types says what that function accepts on that type. In all cases, it is an ISO extended string which corresponds to the portions of the time that type supports. The standard says what ISO extended strings look like, but it does not require that all fields be present in an ISO extended string, and there is no requirement that code parsing an ISO extended string support every variant of ISO extended string in existence, and none of the functions in std.datetime are intended to support every variant of ISO extended string. Not even SysTime.fromISOExtString does, since it doesn't support dates or times on their own (which would be valid ISO extended strings so long as they're formatted correctly for those portions of the date or time). As such, I see no problem with what these functions parse. They do exactly what they say they'll do. That being said, it is true that it's possible to pass an ISO extended string which either has unsupported fields or which is missing required fields to fromISOExtString. So, perhaps the exception's message should say something like "String not in the required format:" instead of "Invalid ISO Extended String:". So, I'll create a PR adjusting the error messages. --

https://issues.dlang.org/show_bug.cgi?id=24704 Dlang Bot <dlang-bot@dlang.rocks> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |pull --- Comment #13 from Dlang Bot <dlang-bot@dlang.rocks> --- @jmdavis updated dlang/phobos pull request #9047 "Fix issue 24704: Improve error messages for from*String in std.datetime." fixing this issue: - Fix Bugzilla issue 24704: Improve error messages for from*String in std.datetime. The documentation for the from*String functions specifies what they accept, and the fromISO*String functions accept ISO or ISO extended strings, but it is technically the case that there are ISO and ISO extended strings which they do not accept (since the standard specifies what the fields should look like for each string but doesn't specify that all fields must be present or that any code parsing them must accept all variants). So, technically, the error messages which have said that the given strings are not valid ISO or ISO extended strings are not necessarily correct. So, this changes the error messages to remove that ambiguity. https://github.com/dlang/phobos/pull/9047 --

https://issues.dlang.org/show_bug.cgi?id=24704 Dlang Bot <dlang-bot@dlang.rocks> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |RESOLVED Resolution|--- |FIXED --- Comment #14 from Dlang Bot <dlang-bot@dlang.rocks> --- dlang/phobos pull request #9047 "Fix issue 24704: Improve error messages for from*String in std.datetime." was merged into master: - 2a3374fb3bbb4c246204139c1393b843aafa2d4f by Jonathan M Davis: Fix Bugzilla issue 24704: Improve error messages for from*String in std.datetime. The documentation for the from*String functions specifies what they accept, and the fromISO*String functions accept ISO or ISO extended strings, but it is technically the case that there are ISO and ISO extended strings which they do not accept (since the standard specifies what the fields should look like for each string but doesn't specify that all fields must be present or that any code parsing them must accept all variants). So, technically, the error messages which have said that the given strings are not valid ISO or ISO extended strings are not necessarily correct. So, this changes the error messages to remove that ambiguity. https://github.com/dlang/phobos/pull/9047 --

Forums