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

Suppress multiple match warning #191

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

DavisVaughan
Copy link
Contributor

This PR makes your package compatible with the next version of dplyr:

The join functions in dplyr (like left_join()) now return a warning by default when a row in x matches multiple rows in y. While this is typical SQL behavior, it is often unexpected during data analysis (many people don't even know it is possible), so we've decided to make this a warning. In dplyr 1.1.0, you silence this warning with multiple = "all". In the meantime, to be compatible with both dev and CRAN dplyr we need to work around broken tests of yours that were expecting no warnings. I've done that by suppressing the warnings in the join calls that caused errors. You can switch to explicitly setting multiple = "all" instead after the dplyr 1.1.0 release, but this should work for both dev and CRAN dplyr in the meantime.

We plan to submit dplyr 1.1.0 on January 27th.

This should be compatible with both dev and CRAN dplyr. It would help us out if you could go ahead and send a patch version of your package to CRAN ahead of time! Thanks!

@polettif
Copy link
Contributor

Thanks for the heads up. Small note, shouldn't suppress_matches_multiple_warning be put around all *_join calls within the package? I can handle this if necessary.

@DavisVaughan
Copy link
Contributor Author

I've only put it around ones that cause an error in the package (I think). This is really just a patch, once dplyr 1.1.0 is out you should remove this and set the multiple = "all" argument of the join function instead.

@polettif
Copy link
Contributor

Ah I see, thanks.

@polettif polettif merged commit 3203e95 into r-transit:master Dec 16, 2022
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

Successfully merging this pull request may close these issues.

2 participants