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

Use alternative variant for list columns in frame_data() #32

Merged
merged 7 commits into from
Mar 8, 2016

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 2, 2016

@hadley: This is your implementation, but it fails one of the existing tests. I have an alternative, longer implementation, which passes everything.

@krlmlr
Copy link
Member Author

krlmlr commented Mar 2, 2016

Tests now pass (after adb9400). You decide.

@codecov-io
Copy link

Current coverage is 96.19%

Merging #32 into master will increase coverage by +0.01% as of e1ed6e6

@@            master     #32   diff @@
======================================
  Files           11      11       
  Stmts          419     420     +1
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            403     404     +1
  Partial          0       0       
  Missed          16      16       

Review entire Coverage Diff as of e1ed6e6

Powered by Codecov. Updated on successful CI builds.

frame_mat <- matrix(frame_rest, ncol = frame_ncol, byrow = TRUE)
frame_col <- lapply(seq_len(ncol(frame_mat)), function(i) {
col <- frame_mat[, i]
if (any(vapply(col, function(x) is.list(x) || length(x) != 1L,
Copy link
Member

Choose a reason for hiding this comment

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

How about pulling this out into is_atomic_scalar()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Extracted needs_list_col() (8ed51d8).

krlmlr added a commit that referenced this pull request Mar 8, 2016
- `frame_data()` now also creates a list column if one of the entries is a list (#32).
@krlmlr krlmlr merged commit 86b86d1 into master Mar 8, 2016
@krlmlr krlmlr deleted the feature/hadley-frame_data branch March 8, 2016 20:52
krlmlr pushed a commit that referenced this pull request Mar 8, 2016
- `frame_data()` now also creates a list column if one of the entries is a list (#32).
- New `rownames_to_column()` and `column_to_rownames()` functions, replace `add_rownames()` (#11, @zhilongjia).
krlmlr pushed a commit that referenced this pull request Mar 10, 2016
- Features
    - New `as_data_frame.table()` with argument `n` to control name of count column (#22, #23).
    - New function `repair_names()` fixes missing and duplicate names (#10, #15, @r2evans).
    - `frame_data()` now also creates a list column if one of the entries is a list (#32).
    - New `rownames_to_column()` and `column_to_rownames()` functions, replace `add_rownames()` (#11, @zhilongjia).
    - Use `tibble` prefix for options (#13, #36).

- Documentation
    - Add pre-tibble NEWS (#39, #40).
    - Include vignette (#38).
    - Expand README.
    - Fix typos in documentation.
    - Remove use of `src()` from examples.

- Prepare CRAN release
    - Use new-style `.travis.yml`
    - Use AppVeyor for testing.
    - Finer coverage analysis (#37).
    - Check with win-builder and valgrind.
    - Fix NOTE from `R CMD check`.
@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.

3 participants