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

grouped_df changes type of vars argument #665

Closed
kasterma opened this issue Oct 7, 2014 · 5 comments
Closed

grouped_df changes type of vars argument #665

kasterma opened this issue Oct 7, 2014 · 5 comments
Assignees
Milestone

Comments

@kasterma
Copy link

kasterma commented Oct 7, 2014

I just did:

install.packages("devtools")
devtools::install_github("hadley/lazyeval")
devtools::install_github("hadley/dplyr")

So this concerns the latest version available on github.

Executing:

library(dplyr)
features <- list("feat1", "feat2", "feat3")
class(features[[1]])
grouped_df(data.frame(feat1=1, feat2=2, feat3=3), features)
class(features[[1]])

Shows that the class of features has changed from "character" to name. Here is a copy paste from an Rstudio repl:

> library(dplyr)

Attaching package: ‘dplyr’

The following object is masked from ‘package:stats’:

    filter

The following objects are masked from ‘package:base’:

    intersect, setdiff, setequal, union

> features <- list("feat1", "feat2", "feat3")
> class(features[[1]])
[1] "character"
> grouped_df(data.frame(feat1=1, feat2=2, feat3=3), features)
Source: local data frame [1 x 3]
Groups: feat1, feat2, feat3

  feat1 feat2 feat3
1     1     2     3
> class(features[[1]])
[1] "name"
> 

From the top of the repl:

R version 3.1.1 (2014-07-10) -- "Sock it to Me"
Copyright (C) 2014 The R Foundation for Statistical Computing
Platform: x86_64-apple-darwin13.1.0 (64-bit)
@romainfrancois
Copy link
Member

As far as I'm concerned, this is a bug in Rcpp from which dplyr gets the ListOf template class used in group_by underlying code. RcppCore/Rcpp#188

@romainfrancois
Copy link
Member

Looking at this some more, I think what we can do is change the code of grouped_df so that it makes a list of symbols before going down to the C++ layer, so that we are immune to the Rcpp bug and/or we don't have to wait for the next Rcpp release.

@hadley
Copy link
Member

hadley commented Oct 7, 2014

Or just throw an error - grouped_df is mostly for internal use.

@romainfrancois
Copy link
Member

Fair enough. This piles up to what @kevinushey and I have been discussing on RcppCore/Rcpp#188

I'll add defensive code in dplyr but I think Rcpp should throw an error, following what Rcpp11 does.

@romainfrancois
Copy link
Member

I've put some defensive code against RcppCore/Rcpp#188, now I get:

> features <- list("feat1", "feat2", "feat3")
> grouped_df(data.frame(feat1=1, feat2=2, feat3=3), features)
Erreur : Elements 1, 2, 3 of sapply(vars, is.name) are not true

@lock lock bot locked as resolved and limited conversation to collaborators Jun 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants