Add Support for DATE_TIMEs in STATA#325
Conversation
evanmiller
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I left some suggestions.
src/bin/read_csv/mod_dta.c
Outdated
| fprintf(stderr, "%s:%d not a valid date-time: %s (expected format: yyyy-mm-dd hh:MM:SS with optional milliseconds. Datetime string is truncated at 23 characters to ignore microseconds and timezone information.)\n", __FILE__, __LINE__, date_time); | ||
| exit(EXIT_FAILURE); | ||
| } | ||
| int missing_ranges_count = readstat_variable_get_missing_ranges_count(var); |
There was a problem hiding this comment.
Why was this logic removed?
There was a problem hiding this comment.
I had initially copied the value_double_dta to value_double_date_time_dta to ensure code was being reached, before implementing value_double_date_time_dta as its own function. I wasn't sure how this code would handle blank date fields in the CSV.
There was a problem hiding this comment.
OK, I'm guessing missing ranges are seldom used with dates, so let's leave it out
|
@evanmiller Thanks for the feedback and suggestions, I'll be the first to admit my C skills are very, very rusty. If there's value in re-inserting the |
|
The macOS build is unhappy for some reason; not sure if related to your change or not |
@evanmiller Any tips on how I can best figure these out? I've been compiling on Rocky 9 but don't have an easy way to test on MacOS. Thanks again for your guidance and expertise. |
|
Looks like a pre-existing error, so I'm not going to sweat it. Thanks for the contribution. |
When converting a CSV to STATA, I received the following error:
src/bin/read_csv/mod_dta.c:404 unsupported variable type 3This PR adds support for DATE_TIMEs to the STATA writer. Here's the test CSV that was used:
Here's the JSON mapping file:
{ "type": "STATA", "variables": [ { "type": "NUMERIC", "name": "orgpermid", "label": "OrgPermID (orgpermid)", "format": "UNSPECIFIED" }, { "type": "NUMERIC", "name": "valuecalcdt", "label": "ValueCalcDt (valuecalcdt)", "format": "DATE_TIME" }, { "type": "NUMERIC", "name": "justdate", "label": "JustDate (justdate)", "format": "DATE" } ] }...and the output in STATA from the written file:
...and to validate millisecond display:
In reviewing the code, I also noticed the patterns for the three DATE_TIME patterns did not match the
Date-Time Unitfrom the source spec here: https://libguides.library.kent.edu/SPSS/DatesTimeI've also changed those patterns to match the spec.