eddelbuettel / rcppsimdjson

Rcpp Bindings for the 'simdjson' Header Library
115 stars 13 forks source link

Change in behavoir of query argument of fparse between version 0.1.1 and 0.1.2 #51

Closed mem48 closed 3 years ago

mem48 commented 3 years ago

I'm updated my package, and it seems the new version of RcppSimdJson has changed whether the query argument needs a leading slash.

For example:

json_string = '{"a":[[1,2,3],["a","b","c"]],"b":[[4,5,6],["d","e","f"]]}'

# Works in 0.1.1, but doesn't work in 0.1.2
RcppSimdJson::fparse(json_string, query = "a") 

# Doesn't work in 0.1.1, but works in 0.1.2
RcppSimdJson::fparse(json_string, query = "/a") 

Is this an intentional permanent change? In which case I need to update my package to require version >= 0.1.2.

eddelbuettel commented 3 years ago

Correct. That comes from upstream in 0.5.0 and there may be another change in 0.6.0 (which I am currently updating to and where we saw a few more (miniscule) changes in error behavior).

You will have to update your code for the leading slash.

/cc @lemire

mem48 commented 3 years ago

Thanks for the clarification.

I've been getting an error on CRAN after updating my package, so I wanted to check that I really had to enforce using version 0.1.2

Version: 0.3.0
Check: package dependencies
Result: ERROR
    Package required but not available: ‘RcppSimdJson’

    See section ‘The DESCRIPTION file’ in the ‘Writing R Extensions’
    manual.
Flavor: r-patched-solaris-x86
eddelbuettel commented 3 years ago

That may be between you and CRAN. There can be occassional race conditions on the builders, this comes up from time to time on the (excellent) r-package-devel list and the recommendation (by Uwe Ligges) is usually to just resubmit (if your're sure it is not you).

Looking at https://cloud.r-project.org/web/packages/RcppSimdJson/index.html does not suggest any binaries are missing.

So on balance I am going to close this. Please feel free to reopen if I overlooked anything.

lemire commented 3 years ago

At some point, some of us misread the specifications (RFC) and this lead to an improper syntax parsing. I am very sorry about this, but... yes, the leading slashes are needed as per JSON Pointer.

Our own syntax was self-consistent, but it did not match anything that was formally published. We were silently innovating (without realizing it).

mem48 commented 3 years ago

Thanks for the speedy and helpful response, and for writing an amazing package.

mem48 commented 3 years ago

Hi @eddelbuettel

Looking at https://cloud.r-project.org/web/packages/RcppSimdJson/index.html I see that the windows binaries for r-oldrel are not available.

I can confirm that I can't install rcppsimdjson on Windows with R 3.6 and it won't compile from source either.

eddelbuettel commented 3 years ago

Yes that is the way it goes with compilers that are too old.

R 4.0.0 was released six months ago now following the usual extensive testing that effectively takes a year during which r-devel ripens, and has by now seen three more patch releases. If you want to benefit from "modern C++" alongside it you need to upgrade and will also get Rtools4 in the process.