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

[R] Two tests are failing with to-be-released dplyr dependency #33666

Closed
paleolimbot opened this issue Jan 15, 2023 · 5 comments · Fixed by #33693
Closed

[R] Two tests are failing with to-be-released dplyr dependency #33666

paleolimbot opened this issue Jan 15, 2023 · 5 comments · Fixed by #33693
Assignees
Labels
Component: R Priority: Blocker Marks a blocker for the release Type: bug
Milestone

Comments

@paleolimbot
Copy link
Member

Describe the bug, including details regarding any error messages, version, and platform.

Error (test-dplyr-join.R:241): semi_join
<rlib_error_dots_nonempty/rlib_error_dots/rlang_error/error/condition>
Error in `semi_join(., to_join, by = "some_grouping", keep = TRUE)`: `...` must be empty.
x Problematic argument:
* keep = TRUE
Backtrace:
 1. arrow:::compare_dplyr_binding(...)
      at test-dplyr-join.R:241:2
 6. dplyr:::semi_join.data.frame(., to_join, by = "some_grouping", keep = TRUE)

(Looks like we just need to remove keep = TRUE, which was never an argument to semi_join() anyway?)

Error (test-dplyr-funcs-datetime.R:3641): with_tz() and force_tz() works
<dplyr:::mutate_error/rlang_error/error/condition>
Error in `mutate(., timestamps_with_tz_1 = force_tz(timestamps, "Europe/Brussels", 
    roll_dst = c("boundary", "post")))`: i In argument: `timestamps_with_tz_1 = force_tz(...)`.
Caused by error in `force_tz()`:
! unused argument (roll_dst = c("boundary", "post"))
Backtrace:
  1. arrow:::compare_dplyr_binding(...)
       at test-dplyr-funcs-datetime.R:3641:2
  6. dplyr:::mutate.data.frame(...)
  7. dplyr:::mutate_cols(.data, dplyr_quosures(...), by)
  9. dplyr:::mutate_col(dots[[i]], data, mask, new_columns)
 10. mask$eval_all_mutate(quo)
 11. dplyr (local) eval()

Component(s)

R

@assignUser
Copy link
Member

assignUser commented Jan 15, 2023

Should this be a blocker? We could cherry pick it into the cran release but eh

@paleolimbot
Copy link
Member Author

It would be nice to not have to cherry pick! I think this one is a quick.

There's also an LTO nightly failure that's probably an arrow C++ problem (also probably a quick fix).

@assignUser assignUser added the Priority: Blocker Marks a blocker for the release label Jan 16, 2023
@assignUser
Copy link
Member

Marked as blocker as worst case it will necessitate another CRAN release

@thisisnic
Copy link
Member

@paleolimbot Where did you spot the second test failure? I couldn't reproduce it locally after installing dev dplyr.

@paleolimbot
Copy link
Member Author

I believe that's because I hadn't installed the latest lubridate on my laptop. We don't have to have our tests pass for outdated lubridate so feel free to ignore it!

@kou kou changed the title Two tests are failing with to-be-released dplyr dependency [R] Two tests are failing with to-be-released dplyr dependency Jan 17, 2023
paleolimbot pushed a commit that referenced this issue Jan 17, 2023
This PR removes the `keep` argument from the test for `semi_join()`, which are causing the unit tests to fail.  It also removes the argument `suffix` argument (which is not part of the dplyr function signature) from the function signature here.

Closes: #33666

Authored-by: Nic Crane <thisisnic@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
raulcd pushed a commit that referenced this issue Jan 18, 2023
This PR removes the `keep` argument from the test for `semi_join()`, which are causing the unit tests to fail.  It also removes the argument `suffix` argument (which is not part of the dplyr function signature) from the function signature here.

Closes: #33666

Authored-by: Nic Crane <thisisnic@gmail.com>
Signed-off-by: Dewey Dunnington <dewey@fishandwhistle.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: R Priority: Blocker Marks a blocker for the release Type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants