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

new example for vignette #23

Closed
momeara opened this issue Aug 5, 2015 · 7 comments
Closed

new example for vignette #23

momeara opened this issue Aug 5, 2015 · 7 comments

Comments

@momeara
Copy link

momeara commented Aug 5, 2015

Hi what a nice package--

Perhaps you'd like to add to your vignette that it's easy to check that a data frame column contains a column

data.frame(a=c(1,2,3)) %>%
    verify("a" %>% exists) %>%    # ok
    verify("b" %>% exists)        # fails
@tonyfischetti
Copy link
Owner

Thanks for the kind words! That would be super useful. Because of the nested magrittr pipes, do you have any idea what exists would look like?

@momeara
Copy link
Author

momeara commented Aug 9, 2015

Exists already exists! If you try it, you'll see it works with the current version of assertr : D

@tonyfischetti
Copy link
Owner

Just checked this. It will return TRUE is there's a variable named the same name as the column to be checked...

mpgg <- "this"
mtcars %>%
  verify("mpgg" %>% exists) %>%
  head # passes

Which can be really dangerous.
So you have to do this instead

mpgg <- "this"
mtcars %>%
  verify(exists("mpgg", where=.)) %>%
  head # fails

I think it might be worth writing a dedicated wrapper function (I'll do it!).
What do you think?

@momeara
Copy link
Author

momeara commented Aug 15, 2015

Nice catch, yes, in this case a wrapper would be helpful.

On Sat, Aug 15, 2015 at 11:48 AM, Tony Fischetti notifications@github.com
wrote:

Just checked this. It will return TRUE is there's a variable named the
same name as the column to be checked...

mpgg <- "this"mtcars %>%
verify("mpgg" %>% exists) %>%
head # passes

Which can be really dangerous.
So you have to do this instead

mpgg <- "this"mtcars %>%
verify(exists("mpgg", where=.)) %>%
head # fails

I think it might be worth writing a dedicated wrapper function (I'll do
it!).
What do you think?


Reply to this email directly or view it on GitHub
#23 (comment)
.

@tonyfischetti
Copy link
Owner

Because I don't have any other "predicates" that can't be used with verify and not the other verbs I don't think I'm gonna write the wrapper function, but I'm gonna put it in the vignette!

tonyfischetti added a commit that referenced this issue Mar 16, 2017
@tonyfischetti
Copy link
Owner

I decided to implement this after all! It's has_all_names :) Thanks for the suggestion!

It's in the README and vignette now, too

@momeara
Copy link
Author

momeara commented Mar 16, 2017

👍

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

No branches or pull requests

2 participants