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

Adapt to the changes in dplyr 0.6.0 #76

Merged
merged 2 commits into from
Apr 28, 2017
Merged

Adapt to the changes in dplyr 0.6.0 #76

merged 2 commits into from
Apr 28, 2017

Conversation

onurfiliz
Copy link
Contributor

The changes:

  • Implement db_desc dispatching on PrestoConnection.
  • Some of the functions in dplyr moved to dbplyr. So we add a dbplyr
    compatibility function which pseudo-imports the function from the
    'correct' package. This way we can provide backwards compatibility.
  • Update the tests to use 'quosures' and the new evaluation framework
    where necessary.

resolves #75.

The changes:
- Implement `db_desc` dispatching on `PrestoConnection`.
- Some of the functions in dplyr moved to `dbplyr`. So we add a `dbplyr`
  compatibility function which pseudo-imports the function from the
  'correct' package. This way we can provide backwards compatibility.
- Update the tests to use 'quosures' and the new evaluation framework
  where necessary.

resolves #75.
While this works with most types, it does not with `raw`.
Either the current form, or:

    r <- as.raw(0)
    rq <- quo(r)

    ...
        as(a, !!rq)
    ...

works.
@facebook-github-bot
Copy link

@onurfiliz updated the pull request - view changes

Copy link
Contributor

@cosinequanon cosinequanon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great, I'm a bit confused about one of the tests but o/w feel free to merge

l <- list(a=1L)
expect_equal(
translate_sql(
as(x, !!l),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about this line, why are we applying !! to a list?

)
expect_equal(
translate_sql(
as(x, !!local(list(a=Sys.time()))),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

@onurfiliz
Copy link
Contributor Author

Honestly I'm not sure why but these have to be UQ()ed whereas the ones in the transmute call do not. I will try to isolate it to a reproducible example and open an issue with dbplyr.

@onurfiliz onurfiliz merged commit 5afe53d into master Apr 28, 2017
@onurfiliz onurfiliz deleted the dplyr_0.6.0_fixes branch April 28, 2017 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test RPresto with dplyr 0.6
3 participants