andrewrech / antigen.garnish

Other
45 stars 13 forks source link

'v %>% data.table::as.data.table' pipe syntax fails in recent R-- parentheses required for namespace-prefixed functions #124

Closed danoreper closed 4 years ago

danoreper commented 4 years ago

Hi all, In R-3.6.1, the following block of code in antigen.garnish::garnish_dissimilarity hits a failure (it used to work for me in an older R version):

 sdt <- v %>% data.table::as.data.table %>% .[, `:=`(nmer_id, names(v))]
# Error in .::data.table : unused argument (as.data.table)

I assumed this was a bug in the latest iteration of pipes, and reported it in dplyr's git repo. However, Hadley Wickham's guidance was as follows: https://github.com/tidyverse/dplyr/issues/5125 In short, the failure is not due to a bug in pipes-- rather, the pipe syntax now requires parentheses at the end for namespace-prefixed functions; e.g., pipe syntax requires: data.table::as.data.table()

Consistent with Hadley's directions, the following statement succeeds:

 sdt <- v %>% data.table::as.data.table() %>% .[, `:=`(nmer_id, names(v))]

It looks like piping into data.table::as.data.table occurs in multiple places in antigen.garnish_predict.R, and perhaps other files as well.

leeprichman commented 4 years ago

Hi danoreper,

Thanks for reaching out. We ran into this previously with CRAN vs development version magrittr where the dated version was not parsing correctly without the trailing parentheses. We have a version check in the package but that seems to be failing in this new circumstance.

I will work on remedying this throughout the code base. Can you send me your sessioninfo when you get a chance just so I can get a sense of differences? As well, if you aren't using it already, installing the dev level magrittr version from github may be a temporary fix so that you can keep working.

leeprichman commented 4 years ago

4b19c32

That commit should take care of it. Please let me know whether or not it is working for you when you get a chance.

danoreper commented 4 years ago

Hi Lee, Thanks for the quick response! I apologize for my delay, a ton of stuff came up this week.

I've checked out the sha1 above: git checkout 4b19c32

but it looks like there are still data.table::as.data.table invocations in antigen.garnish_predict.R; I installed the checked-out version and got the same errors.

Checking the file diff manually, via: git diff 4b19c32..master ./R/antigen.garnish_predict.R > diffs.txt See: diffs.txt

I see no changes to data.table::as.data.table syntax.

Is it possible it's a different SHA1 that I should be using? Am I overlooking something? Thanks, Dan

leeprichman commented 4 years ago

Left newline out of my regex and didn't catch them all. I will push something this morning. Thank you for catching this and sorry for the inconvenience!

leeprichman commented 4 years ago

e0c1e69

Caught additional instances of piping (%>% and %<>%) to namespaced function calls after newline in scripts and. tests. This should resolve it, I am testing myself as well now. Please let me know if you have more problems.

The a.g magrittr version check was still erring for me and prompting me to install with devtools, so I'm not sure why you haven't gotten those warnings. Can you share your sessionInfo() when you get a chance?

leeprichman commented 4 years ago

Above commit should have resolved this and I haven't run into problems. Closing this for now. Please feel free to comment, reopen an issue, or email me if the problem persists.