corybrunson / ggalluvial

ggplot2 extension for alluvial plots
http://corybrunson.github.io/ggalluvial/
GNU General Public License v3.0
499 stars 34 forks source link

Not supporting the dplyr 1.0.10 version #107

Closed MarinkavP closed 1 year ago

MarinkavP commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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.

MarinkavP commented 1 year ago

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

corybrunson commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

Am Fri, Feb 24, 2023 at 05:49:58AM -0800 schrieb Cory Brunson:

@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?

As I said: On my local box its perfectly the same for version 0.12.5.

corybrunson commented 1 year ago

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 commented 1 year ago

Am Fri, Feb 24, 2023 at 02:36:38PM -0800 schrieb Cory Brunson:

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. I'm a bit offline-ish this weekend and have overridden the log by one that is forcing dplyr 1.1.0 (which is now also the case in online available logs at Debian sites). I wonder what might happen at your side if you install 1.0.10 and run the test suite of ggalluvial? If this does not reproduce the problem I described I can try again to do it locally again and recreate the problem. Kind regards, Andreas.

corybrunson commented 1 year ago

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 commented 1 year ago

Am Sat, Feb 25, 2023 at 07:01:56AM -0800 schrieb Cory Brunson:

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. Seems I made some mistake when testing 0.12.5 in connection with latest dplyr. I now uploaded the Debian package where I dropped the restriction on dplyr version and the relevant logs are showing success. So from my point of view the issue can be closed. Kind regards, Andreas.

corybrunson commented 1 year ago

Excellent! Thank you very much for checking.

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