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

Not supporting the dplyr 1.0.10 version #107

Closed
MarinkavP opened this issue Feb 1, 2023 · 16 comments
Closed

Not supporting the dplyr 1.0.10 version #107

MarinkavP opened this issue Feb 1, 2023 · 16 comments
Projects

Comments

@MarinkavP
Copy link

The ggalluvial doesn't work well with the newest update of dplyr 1.0.10 version. I get a warning and certain parts of the graphs are missing, depending on the graphs.

Reproducible example (preferably using reprex::reprex())

data(majors)
majors$curriculum <- as.factor(majors$curriculum)
ggplot(majors,
aes(x = semester, stratum = curriculum, alluvium = student,
fill = curriculum, label = curriculum)) +
scale_fill_brewer(type = "qual", palette = "Set2") +
geom_flow(stat = "alluvium", lode.guidance = "frontback",
color = "darkgray") +
geom_stratum() +
theme(legend.position = "bottom") +
ggtitle("student curricula across several semesters")

Warning message:
Computation failed in stat_stratum():
could not find function "default_missing"

@corybrunson
Copy link
Owner

Hi @MarinkavP, thanks for raising the issue!

Could you check the versions of {dplyr} and of {ggalluvial} you have installed? I'm using {dplyr} 1.0.10 and {ggalluvial} 0.12.3 and was not able to reproduce the warning. (I see that {dplyr} 1.1.0 is on CRAN, but i only last updated my packages before it arrived on January 29.) (It would be especially helpful to have the example generated using the {reprex} package.)

@MarinkavP
Copy link
Author

So I went back and forth between different dplyr versions to determine what causes the issue. The problem happens when I use {dplyr} 1.1.0, it I go back to {dplyr} 1.0.10 the problem is gone. I also had to go back a version of tidyr because they depend on each other. I can create an example when I am at work again in 12hours.

Also wanted to thank for this package. The graphs are so nice and makes it so much easier to explain my dataset.

@corybrunson
Copy link
Owner

Aha, thank you! Yes, this issue seems to have begun with {dplyr} 1.1.0, and i'm sure it has to do with changes to the first and last functions; see the summary of changes. It seems to have already been resolved in the development version. You should be able to resolve the problem by installing {ggalluvial} from GitHub. See the README for instructions.

The package hasn't been updated on CRAN in over a year, so i'll take this opportunity to. I'm not ready to merge in the pivots branch as i'd hoped to, but such is life. I'll close this issue once the next release is out and the fix is confirmed.

@corybrunson corybrunson added this to To do in v1.0.0 via automation Feb 1, 2023
@corybrunson
Copy link
Owner

corybrunson commented Feb 5, 2023

@MarinkavP i just installed the new patch and confirmed that it solves the problem. However...

When run using an older version of {dplyr}, a different bug occurs that produces the same problem with the plot. I should have tested this possibility before releasing the patch. I'll specify dplyr >= 1.1.0 in the DESCRIPTION, but i'm not sure i want to resubmit to CRAN in order to ensure that this gets flagged when new users install {ggalluvial} without upgrading {dplyr}. What do you think?

Anyway, i'll leave this issue open to refer such users to.

corybrunson added a commit that referenced this issue Feb 5, 2023
@MarinkavP
Copy link
Author

I think this is fine, thanks anyway for the fast update!

@corybrunson
Copy link
Owner

@MarinkavP if and when you install version 0.12.5 from CRAN (regardless of your {dplyr} version), could you confirm that this is no longer a problem?

@tillea
Copy link

tillea commented Feb 24, 2023

BTW, the Debian packaged ggalluvial version 0.12.4. is featuring errors inside the test suite (seek for "FAIL" in this long log) in connection with dplyr 1.0.10. I realised that when using dplyr 1.11.0 these test suite errors are gone. That's why I was forcing a versioned Depends on ggalluvial to dplyr >= 1.11.0. I would be happy if someone would confirm that all those dplyr dependency issues will be gone with version 0.12.5.
I can confirm that the test suite error in connection with dplyr 1.0.10 persists also in version 0.12.5 of ggalluvial.

@corybrunson
Copy link
Owner

@tillea i apologize; i don't understand the request.

I've installed {dplyr} 1.0.10 on my machine, and it works fine with {ggalluvial} 0.12.5. The errors at the link are documented in #108, which were resolved for the user who raised the issue with 0.12.5.

I would be glad to run additional tests, or explain in detail the source of the error, how it was resolved, and my understanding of why 0.12.5 should no longer produce it. If you share an error suite using 0.12.5, then i will take a close look and try to diagnose it.

@tillea
Copy link

tillea commented Feb 24, 2023

The point is of the erroneous log is:

...
Get:95 http://deb.debian.org/debian testing/main amd64 r-cran-dplyr amd64 1.0.10-1 [1,227 kB]
...
[ FAIL 5 | WARN 2 | SKIP 11 | PASS 38 ]

══ Skipped tests ═══════════════════════════════════════════════════════════════
• alluvial cannot be loaded (6)
• On CRAN (5)

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-stat-flow.r:10'): `stat_flow` weights computed variables but drops weight ──
Error in `summarise(.tbl, !!!funs)`: Problem while computing `lode = (function (x, order_by = NULL, default =
NULL, na_rm = FALSE) ...`.
ℹ The error occurred in group 1: alluvium = 1, x = 1, yneg = FALSE, stratum =
  "B", deposit = 1, fissure = 1, link = 1, flow = from, adj_deposit = 1.1.3,
  adj_fissure = 1.1.1.
Caused by error in `nth()`:
! unused argument (na_rm = na_rm)
Backtrace:
     ▆
  1. ├─base::as.data.frame(StatFlow$compute_panel(data)) at test-stat-flow.r:10:2
  2. ├─StatFlow$compute_panel(data)
  3. │ └─ggalluvial (local) compute_panel(..., self = self)
  4. │   └─dplyr::summarize_at(data, "lode", distill)
  5. │     ├─dplyr::summarise(.tbl, !!!funs)
  6. │     └─dplyr:::summarise.grouped_df(.tbl, !!!funs)
  7. │       └─dplyr:::summarise_cols(.data, dplyr_quosures(...), caller_env = caller_env())
  8. │         ├─base::withCallingHandlers(...)
  9. │         └─dplyr:::map(quosures, summarise_eval_one, mask = mask)
 10. │           └─base::lapply(.x, .f, ...)
 11. │             └─dplyr (local) FUN(X[[i]], ...)
 12. │               └─mask$eval_all_summarise(quo)
 13. ├─dplyr (local) `<fn>`(lode)
 14. └─base::.handleSimpleError(...)
 15.   └─dplyr (local) h(simpleError(msg, call))
 16.     └─rlang::abort(bullets, call = error_call, parent = skip_internal_condition(e))
── Error ('test-stat-flow.r:28'): `stat_flow` orders flows correctly with negative values ──
Error in `summarise(.tbl, !!!funs)`: Problem while computing `lode = (function (x, order_by = NULL, default =
NULL, na_rm = FALSE) ...`.
ℹ The error occurred in group 1: alluvium = 1, x = 1, yneg = FALSE, stratum =
  "A", deposit = 2, fissure = 1, link = 1, flow = from, adj_deposit = 1.2.3,
  adj_fissure = 1.1.1.
Caused by error in `nth()`:
! unused argument (na_rm = na_rm)
Backtrace:
     ▆
  1. ├─StatFlow$compute_panel(data) at test-stat-flow.r:28:2
  2. │ └─ggalluvial (local) compute_panel(..., self = self)
  3. │   └─dplyr::summarize_at(data, "lode", distill)
  4. │     ├─dplyr::summarise(.tbl, !!!funs)
  5. │     └─dplyr:::summarise.grouped_df(.tbl, !!!funs)
  6. │       └─dplyr:::summarise_cols(.data, dplyr_quosures(...), caller_env = caller_env())
  7. │         ├─base::withCallingHandlers(...)
  8. │         └─dplyr:::map(quosures, summarise_eval_one, mask = mask)
  9. │           └─base::lapply(.x, .f, ...)
 10. │             └─dplyr (local) FUN(X[[i]], ...)
 11. │               └─mask$eval_all_summarise(quo)
 12. ├─dplyr (local) `<fn>`(lode)
 13. └─base::.handleSimpleError(...)
 14.   └─dplyr (local) h(simpleError(msg, call))
 15.     └─rlang::abort(bullets, call = error_call, parent = skip_internal_condition(e))
── Error ('test-stat-flow.r:56'): `stat_flow` orders alluvia correctly according to `aes.bind` ──
Error in `summarise(.tbl, !!!funs)`: Problem while computing `lode = (function (x, order_by = NULL, default =
NULL, na_rm = FALSE) ...`.
ℹ The error occurred in group 1: alluvium = 1, x = 1, yneg = FALSE, stratum =
  "A", deposit = 2, fissure = -2, link = 1, flow = from, adj_deposit = 1.2.4,
  adj_fissure = 1.-2.-2.
Caused by error in `nth()`:
! unused argument (na_rm = na_rm)
Backtrace:
...

If in contrast

Get:96 http://deb.debian.org/debian unstable/main amd64 r-cran-dplyr amd64 1.1.0-1 [1,505 kB]

is used as you can see in the Debian CI log the test passes. On my local machine exchanging the r-cran-dplyr package is the only change which decides about success or failure of the test in ggalluvial package and this is reproducible.
Hope this clarifies the problem.

@corybrunson
Copy link
Owner

@tillea thanks a lot. From the log, it looks like the {ggalluvial} installation is 0.12.4. Could you try this with 0.12.5? Or am i missing something?

@tillea
Copy link

tillea commented Feb 24, 2023 via email

@corybrunson
Copy link
Owner

Could you share the log from your local computer? The passing tests you link to above use {ggalluvial} 0.12.5 as well as {dplyr} {1.1.0-1). So i just want to see the output for a failed test using 0.12.5. At this point i don't see how the same error message could arise with 0.12.5 installed.

@tillea
Copy link

tillea commented Feb 25, 2023 via email

@corybrunson
Copy link
Owner

This is what i meant to say that i did a few comments back: I installed {dplyr} 1.0.10 and {ggalluvial} 0.12.5 on my machine and ran a full check, including tests. I got no errors or failed tests. So yes, at your convenience, please do see if you can reproduce the errors using 0.12.5. I'll take a close look then.

@tillea
Copy link

tillea commented Feb 26, 2023 via email

@corybrunson
Copy link
Owner

Excellent! Thank you very much for checking.

@MarinkavP if you encounter this problem again, please feel free to reopen the issue.

v1.0.0 automation moved this from To do to Done Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
v1.0.0
  
Done
Development

No branches or pull requests

3 participants