Skip to content

C++ Datetime Parsing#1220

Merged
texodus merged 3 commits intomasterfrom
cpp-date-parser
Oct 12, 2020
Merged

C++ Datetime Parsing#1220
texodus merged 3 commits intomasterfrom
cpp-date-parser

Conversation

@texodus
Copy link
Member

@texodus texodus commented Oct 11, 2020

Re-implement string parsing for datetime and date perspective types in C++ via the date.h and Apache Arrow libraries. With this change, moment.js is no longer a requirement of @finos/perspective, shaving ~80kb from the built asset; however, this feature currently is only compatible with Arrow 1.0.1, which means Python cannot use it yet. In the interim, functions such as make_view() still take a date_parser argument to maintain C++ API compatibility, but this argument will be ignored in the WebAssembly compiled engine.

  • RFC 2282 support removed. What other than web metrics uses this format?
  • hh:mm:ss.SSS format removed. This is relatively simple to add back, but it would be nice to settle on the semantics of date-relative formats like this, first.

Also addresses two unrelated bugs in the new Arrow-based CSV parser, encountered during testing:

  • Added UnixTimestamptParser, as perspective's to_csv() outputs datetimes in this format (though we should likely remedy this ..). This parser is not used for inference, so a column must be already typed "date" or "datetime"; otherwise a column of timestamps will be inferred as type "integer".
  • Fixed string encoding of to_csv() to escape strings with " characters.

@texodus texodus added C++ internal Internal refactoring and code quality improvement JS labels Oct 11, 2020
@texodus texodus marked this pull request as ready for review October 12, 2020 01:06
@texodus texodus merged commit b78bd83 into master Oct 12, 2020
@texodus texodus deleted the cpp-date-parser branch October 12, 2020 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ internal Internal refactoring and code quality improvement JS

Development

Successfully merging this pull request may close these issues.

1 participant