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

Use relationship on dplyr >=1.1.0.9000 #8

Closed

Conversation

DavisVaughan
Copy link
Contributor

We are preparing to release dplyr 1.1.1 and this package popped up in the revdep checks.

We realized that we didn't get the multiple argument quite right the first time around 😞 . It was too aggressive since it warned on both one-to-many and many-to-many joins. In 1.1.1 we've made two improvements:

  • multiple now defaults to "all". The options of NULL, "error", and "warning" are deprecated.
  • relationship is a new argument to add known constraints onto the join procedure, such as "many-to-one", which replaces multiple = "error".

The default of relationship checks to see if there is a many-to-many relationship between the keys of x and y and will warn if one is present. This should be much rarer than what we checked for before, and targets the most dangerous case that we were trying to warn the user about.

You can read all about relationship here tidyverse/dplyr#6753, along with the issues linked there.

Unfortunately it does affect some tests here. You are doing one of the few cases where a many-to-many join is reasonable (i.e. a kind of self-join between cr to itself), but this is still pretty rare. I think the easiest thing to do is to just branch off the dplyr version that is installed and set relationship = "many-to-many" or multiple = "all" accordingly.

We plan to submit dplyr 1.1.1 in 2-3 weeks.

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!

@echasnovski
Copy link
Owner

It is indeed hard to come up with versatile updates, isn't it? :(

Thanks, I'll look into it and submit to CRAN (hopefully soon enough).

@echasnovski
Copy link
Owner

Thank you, Davis, for this heads up!

I've merged it and attempt CRAN release soon.

echasnovski added a commit that referenced this pull request Feb 28, 2023
"many-to-many" joins.

Details:
- Resolves #8.

Co-authored-by: Evgeni Chasnovski <[email protected]>
@echasnovski
Copy link
Owner

@DavisVaughan, 'comperes' with this update is on CRAN now. Apart from having ERROR Installation failed on Windows (which is odd, because it passed both win-builder and GitHub Action), all seems to be fine. Thanks again for this PR!

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