eddelbuettel / rcppsimdjson

Rcpp Bindings for the 'simdjson' Header Library
117 stars 14 forks source link

Feature/deserialize #20

Closed knapply closed 4 years ago

knapply commented 4 years ago

Many refinements following some real-world usage:

Fingers crossed, but I feel more confident this is sustainable.

Last thoughts:

knapply commented 4 years ago

Nothing actually requires it and I should've done it separately.

Initially it was another sanity check that everything not only works, but works with the updates upstream... and then I left it in.

I have vectorized (in the R sense) versions to parse multiple strings and multiple files ready to go to maximize the parser efficiency (instead of creating a new one over and over w/ lapply()), but it seems something went funky with reusing the parser to read files. I reproduced and opened a simdjson issue here: https://github.com/simdjson/simdjson/issues/938.

It should've done the sync separately. That's 100% my bad.

eddelbuettel commented 4 years ago

It should've done the sync separately. That's 100% my bad.

Stuff happens. Do you just want to drop another commit on top and restore two two files?

knapply commented 4 years ago

Yes, I'll get it sorted ASAP.

eddelbuettel commented 4 years ago

Sounds good, and no need to rush.

lemire commented 4 years ago

The upstream issue has been reproduced. It should be "easy" to fix. :-) We will fix it before release 0.4 (which is coming soon).

cc @jkeiser

knapply commented 4 years ago

@lemire Your response time is amazing. We all really appreciate it.

@eddelbuettel The previous file versions have been restored and vectorized versions of .deserialize_json() and .load_json() with tests/docs are in.

eddelbuettel commented 4 years ago

I'll merge. It is still only from your branch off a fork of this into a branch here so we do need another pass anyway before any of this becomes "real".