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

resets plyr attach hook when detaching dplyr #3359

Merged
merged 2 commits into from Feb 28, 2018

Conversation

@jwnorman
Copy link
Contributor

@jwnorman jwnorman commented Feb 13, 2018

When loading and attaching plyr after dplyr, certain functions in dplyr are masked by some in plyr, which can create problems (hence the "You have loaded plyr after dplyr..." message). But when you detach and unload both packages and then load and attach plyr by itself, the message is still printed even though dplyr is not attached nor loaded.

This PR adds an .onDetach() hook which replaces the plyr attach hook function with NULL so that the message isn't given when plyr is safely loaded and attached.

I tested this with @lgbarrett4 by rebuilding dplyr with the .onDetach function and made sure the loading and attaching of plyr by itself worked without a warning message.

krlmlr
krlmlr approved these changes Feb 28, 2018
Copy link
Member

@krlmlr krlmlr left a comment

Thanks! Could you please add a bullet to NEWS?

@jwnorman jwnorman force-pushed the dplyr_on_detach branch from dbb658b to c838c0e Feb 28, 2018
@jwnorman
Copy link
Contributor Author

@jwnorman jwnorman commented Feb 28, 2018

Bullet added 👍

@krlmlr krlmlr merged commit 1aa3db4 into tidyverse:master Feb 28, 2018
0 of 2 checks passed
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Feb 28, 2018

Thanks!

@jwnorman jwnorman deleted the dplyr_on_detach branch Feb 28, 2018
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 11, 2018

@jwnorman: I'm seeing check failures, could you please take a look? https://travis-ci.org/tidyverse/dplyr/jobs/352101385#L1740

@jwnorman
Copy link
Contributor Author

@jwnorman jwnorman commented Mar 12, 2018

will do

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Mar 12, 2018

Thanks, @romainfrancois has fixed it today.

@lock
Copy link

@lock lock bot commented Sep 8, 2018

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Sep 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants