DSL feedback

[sckott]

Hi all, If you have a bit of extra time, can you test out the jqr high level DSL https://github.com/ropensci/jqr#high-level ? We'd like to make sure the DSL is sane before pushing 1st version to CRAN

@jennybc @karthik @cboettig @smbache @cpsievert

[smbache]

When do you plan on submission?

[sckott]

No date in mind, prob. not until early Jan

[smbache]

@sckott roger!

[jennybc]

I am assigning this to myself or else I will forget. I can't do it now.

[sckott]

just a friendly reminder now that we're past the holidays

@jennybc @karthik @cboettig @smbache @cpsievert

thanks all! setting milestone date for Feb 1

[smbache]

Just a few thoughts, after some playing around (take it for what it is, I'm not too familiar with everything)

• Would it make sense to have jq generic (with jq.jqr and jq.default), rather than having jq and jq_?
• Should the result of jq and jq_ both be json, rather than json and character, respectively?
• json %>% dotstr("key[]") is not what I would expect, and different from json %>% dotstr(key[]). Is there a SE way of using e.g. dotstr? Would it make sense to allow formula syntax, ala dplyr's verb_ functions?
• It's perhaps not obvious that %>% fun only executes jq at the last pipe. E.g. json %>% dotstr(key) %>% str. The behaviour here is also a bit weird: (json %>% keys) %>% str, and may indicate a small issue with my implementation of the pipe activation feature...

[smbache]

@sckott I think you should change

list(is_piped = is_piped,
     env      = if (is_piped) sys.frames()[[min(which(is_magrittr_env))]])

to

list(is_piped = is_piped,
     env      = if (is_piped) sys.frames()[[max(which(is_magrittr_env))]])

(i.e. the min should be changed to max...)

in the pipeline_info (in pipe_helpers.R)

Which would fix the nested pipe issue I mentioned above. Not able to make PR just now..

[smbache]

I fixed it and implemented some of the suggestions I made in this PR: https://github.com/ropensci/jqr/pull/25

[sckott]

Thanks so much @smbache ! (was working on response...thanks for the PR too)

Should the result of jq and jq_ both be json, rather than json and character, respectively?

Probably yes. What do you think @richfitz Looks like you changed in the PR - will check that out

It's perhaps not obvious that %>% fun only executes jq at the last pipe. E.g. json %>% dotstr(key) %>% str. The behaviour here is also a bit weird: (json %>% keys) %>% str, and may indicate a small issue with my implementation of the pipe activation feature...

Agree, that behavior is not ideal. Just so we're talking about the same thing, are you getting:

('{"foo": 5, "bar": 8}' %>% keys) %>% str
#> List of 2
#>  $ data: chr "{\"foo\": 5, \"bar\": 8}"
#>  $ args:List of 1
#>   ..$ : atomic [1:1] keys
#>   .. ..- attr(*, "type")= chr "keys"
#>  - attr(*, "class")= chr "jqr"
#>  Error: expecting a string 

the min should be changed to max

Thanks, changed in your PR

[smbache]

Yes, same behavior: it was fetching the wrong pipe, in case of nested pipes. It makes sense, I was mentally reversing the order of sys.frames(). It's fixed in the PR.

The tests and examples seem to run with the new generic approach, so I think it's ok, but you should of course only use it you agree it's cleaner.

My initial comment in the PR (which I deleted) about SE, made no sense, I think. My concern was eg with dotstr where I did not know how to append brackets in an SE way (but I suppose that's what index is for.

Let me know if there's anything else I can help with or clarify.