Add read/write methods for date, time, and datetime types #8787
Conversation
Add `readWriteThis` methods for the date, time, datetime types. Add a test to ensure that the methods can be called and that they output and input matching values when appropriate.
|
Go David Go!! |
|
This issue is important. Thanks! |
modules/standard/DateTime.chpl
Outdated
| proc date.readWriteThis(f) { | ||
| var dash = new ioLiteral("-"); | ||
| f <~> new ioLiteral("{") <~> chpl_year <~> dash <~> chpl_month <~> dash | ||
| <~> chpl_day <~> new ioLiteral("}"); |
There was a problem hiding this comment.
Is there a specific reason for the { } around the date? I'd think 2018-03-12 is clear enough as a date already. Are you following some Python precedent, e.g.?
There was a problem hiding this comment.
I was following the default behavior of Chapel classes, which doesn't make a whole lot of sense given that these are records. I'll remove the curly braces.
modules/standard/DateTime.chpl
Outdated
| var colon = new ioLiteral(":"); | ||
| f <~> new ioLiteral("{") <~> chpl_hour <~> colon <~> chpl_minute <~> colon | ||
| <~> chpl_second <~> new ioLiteral(".") <~> chpl_microsecond | ||
| <~> new ioLiteral("}"); |
| @@ -0,0 +1,3 @@ | |||
| datetime: true | |||
| date: true | |||
| time: true | |||
There was a problem hiding this comment.
Do we have any tests that actually show the date-time formats in the .good file? Might be worth adding one so we can be sure it doesn't accidentally change.
|
Should this be following ISO 8601? |
|
Also I think the original issue, and your test, are about JSON format I/O - but a date field in JSON presumably needs to be inside double quotes - i.e. represented as a string in JSON since JSON doesn't support dates/times directly. |
|
@mppf you probably have a point. I'm not sure the proper way to set it. |
|
The readWriteThis methods can check for json like so: var binary = f.binary();
var arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY);
var isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary; |
This means zero-padding the year to 4 digits and everything else to 2 digits, plus putting a 'T' between the date and time, right? The padding sounds fine, but I'm not crazy about the 'T'. |
I think so... and generally agree. But at least ISO 8601 is a standard. |
Zero-pad years to 4 digits and other fields to 2 digits. Make date and time be separated by a 'T'. Make dates/times/datetimes a string when reading or writing for JSON. Added a test that prints a date, time, and datetime value to lock in the format.
|
Looking good, thanks. |
|
Thank you @mppf! |
Add
readWriteThismethods for the date, time, datetime types. Add a test toensure that the methods can be called and that they output and input matching
values when appropriate.