DavisVaughan commented 1 year ago

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 https://github.com/tidyverse/dplyr/pull/6753, along with the issues linked there.

Unfortunately it does affect some code here, but I think it does so in a positive way!

You can use a non-equi join in set_dates_service() which makes it faster and simpler
The inner_join() in get_trip_geometry() no longer needs multiple to be set. It seems like it was doing a one-to-many join before, which we no longer warn on. But we do have to branch on the dplyr version for this one to support CRAN and dev dplyr.

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!

codecov-commenter commented 1 year ago

Codecov Report

Merging #197 (53233fd) into master (bc8cf53) will decrease coverage by 0.10%. The diff coverage is 85.71%.

:exclamation: Current head 53233fd differs from pull request most recent head 14b2dc8. Consider uploading reports for the commit 14b2dc8 to get more accurate results

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##            master     #197      +/-   ##
===========================================
- Coverage   100.00%   99.90%   -0.10%     
===========================================
  Files           15       15              
  Lines         1062     1057       -5     
===========================================
- Hits          1062     1056       -6     
- Misses           0        1       +1

Impacted Files	Coverage Δ
R/utils.R	`100.00% <ø> (ø)`
R/spatial.R	`98.85% <66.66%> (-1.15%)`	:arrow_down:
R/dates.R	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

polettif commented 1 year ago

Thank you 👍

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!

We'll package a release with some other developments but I think it should work out timewise.

DavisVaughan commented 1 year ago

👍 looking like March 13 for the dplyr release

r-transit / tidytransit

Fix for dplyr 1.1.1 #197

Codecov Report