Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ Datetime Parsing #1220

Merged
merged 3 commits into from
Oct 12, 2020
Merged

C++ Datetime Parsing #1220

merged 3 commits into from
Oct 12, 2020

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant