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

as_data_frame.NULL #16

Merged
merged 1 commit into from
Jan 5, 2016
Merged

as_data_frame.NULL #16

merged 1 commit into from
Jan 5, 2016

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Jan 5, 2016

So that dplyr::tbl_df(NULL) (or tibble::tbl_df(NULL) rather) is NULL instead of an error.

If I understand what's going on, it makes more sense to do this here than in dplyr itself (?).

@codecov-io
Copy link

Current coverage is 96.94%

Merging #16 into master will not affect coverage as of b910386

@@            master     #16   diff @@
======================================
  Files           11      11       
  Stmts          393     393       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            381     381       
  Partial          0       0       
  Missed          12      12       

Review entire Coverage Diff as of b910386

Powered by Codecov. Updated on successful CI builds.

krlmlr added a commit that referenced this pull request Jan 5, 2016
- `as_data_frame(NULL)` returns `NULL` (#16, @jennybc)
@krlmlr krlmlr merged commit 4e6e0c2 into tidyverse:master Jan 5, 2016
@krlmlr
Copy link
Member

krlmlr commented Jan 5, 2016

Thanks! Eventually, dplyr will use this package (tidyverse/dplyr#1595), but tibble probably needs to be on CRAN first.

krlmlr pushed a commit that referenced this pull request Jan 5, 2016
- Use C++ implementation for `as_data_frame.matrix()` (#14)
- `as_data_frame(NULL)` returns `NULL` (#16, @jennybc)
@hadley
Copy link
Member

hadley commented Jan 5, 2016

I don't think this is quite right - I think it should continue to return a data frame, but it should be a 0-row 0-col data frame, i.e. dplyr::data_frame()

@krlmlr
Copy link
Member

krlmlr commented Jan 5, 2016

Agreed, that's also what as.data.frame() does:

> as.data.frame(NULL)
data frame with 0 columns and 0 rows

@jennybc: Would you like to update the code?

@jennybc
Copy link
Member Author

jennybc commented Jan 5, 2016

OK I will change that. I looked for something to emulate and could only find a NULL method for do_(), but I see that it's different.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants