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

Apply drive_id class to the id variable in a dribble #93

Closed
jennybc opened this issue Jun 14, 2017 · 2 comments · Fixed by #364
Closed

Apply drive_id class to the id variable in a dribble #93

jennybc opened this issue Jun 14, 2017 · 2 comments · Fixed by #364
Labels

Comments

@jennybc
Copy link
Member

jennybc commented Jun 14, 2017

It just occurred to me that the id variable in a dribble could have type drive_id. We already have this class and we can apply it inside the functions we already have for creating dribbles. This makes it more attractive to write a format method for drive_id objects that truncates / ellipsizes the ids. This is basically a revival of #5.

@LucyMcGowan
Copy link
Member

Working on this this morning - when do you think would be the most natural time to introduce this class for as_dribble.data.frame() -- if passed a list, it is natural to assign the id a class, but for data frames currently we pass it through a new_dribble() function (that doesn't check any columns, just assigns the class) and then dribble_validate(). My inclination is new_dribble(), but then we'd be semi-validating it before it's been passed to dribble_validate() (to at least make sure it has a column id and assign it class drive_id.

@jennybc
Copy link
Member Author

jennybc commented Jun 15, 2017

I'm not sure when to apply the class. The drive_id formatting isn't going to work in the near future anyway (tidyverse/tibble#212), so let's wait and see what the world looks like once that's cleared up and we're motivated to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants