almartin82 / mapvizieR

visualizations and reports for the NWEA MAP assessment in R
Other
17 stars 6 forks source link

write dplyr verb methods for mapvizier sub-classes (cdf, growth df) #304

Open almartin82 opened 8 years ago

almartin82 commented 8 years ago

per this comment and hadley's comment on dplyr, we write our own wrappers for the dplyr verbs.

thinking about this a bit, my one concern here is dispatch... because of namespace collisions in the tidyverse, lots of people (myself included) have gotten into the habit of being explicit about our dplyr calls... if an end user calls dplyr::select() on a mapviz$cdf object, it will still lose its class... but at least our summary methods will preserve the class of the object, because we can be explicit about using our internal wrappers? thinking out loud here.

almartin82 commented 7 years ago

Hi @jwdink, I've found your advice about this problem on this thread very helpful!

I wrote up a toy question here to try to get a clear worked example on custom dplyr method dispatch. If you have a second, would you be able to take a look? Thanks!

almartin82 commented 7 years ago

...and we've got a solution! thanks all.

almartin82 commented 7 years ago

verbs to clean up

almartin82 commented 7 years ago

closed, finally :100:

jwdink commented 7 years ago

Don't forget dplyr::slice!

almartin82 commented 7 years ago

Thank you!

almartin82 commented 7 years ago

@chrishaid there's been some great new writeups on this issue here and here by @jwdink. Jacob - thank you!

it seems like we need to:

jwdink commented 7 years ago

Oops! FYI @almartin82, I just spotted a problem. You shouldn't use preservatively unless this can be sorted out.

Preservatively assumes the first argument is x:

preservatively <- function(fun) {
  function(x, ...) {
    result <- NextMethod()
    reclass(x, result)
  }
}

But most dplyr verbs have .data as their first arg. If the user calls the verb with an unnamed first argument, there's no issue. But if they call, e.g., filter(.data=foo, condition), then preservatively gets confused.

Changing that x in preservatively to .data would fix the problem for most dplyr verbs, but I think this issue suggests an adverb may not be appropriate after all.

I'll let you know if I figure something out.

It looks like the latest version of Advanced R addresses this issue (reconstruct is similar to reclass). See http://adv-r.hadley.nz/s3.html#methods