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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #407. unnest works with S3 vectors. #419

Closed
wants to merge 2 commits into from
Closed

Conversation

@zeehio
Copy link
Contributor

@zeehio zeehio commented Feb 16, 2018

This PR fixes #407 by using dplyr::combine() to combine factors, dates and other supported S3 vector types.

dplyr does not offer a way to know if dplyr::combine() will succeed based on the types of the vector given, so we use a tryCatch to capture the error and report accordingly. Maybe a better solution will appear when r-lib/vctrs#7 comes to reality.

Apologies if anyone else was working on this issue. I was affected by it, and given my previous work on dplyr::combine() and the Collecter class from dplyr (tidyverse/dplyr#2209, tidyverse/dplyr#2487) I thought this was a low hanging fruit for me. I can close the PR if anyone has a better solution in mind.

Thanks for your time 馃槃

zeehio added 2 commits Feb 16, 2018
The type coercion of S3 vectors relies on dplyr::combine.

dplyr does not offer a way to know if dplyr::combine will succeed based on the types of the vector given, so we use a tryCatch for now.
@hadley
Copy link
Member

@hadley hadley commented Feb 19, 2018

I went with something a little simpler, splitting the difference with the original code: 96ded3f

@hadley hadley closed this Feb 19, 2018
@zeehio zeehio deleted the zeehio:fix_407 branch Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants