plotly / plotly/plotly.R

plotly failures with dev rlang

Open
#2,488 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
R
Stars
2.7k
Forks
641
PR merge metrics
No merged PRs in 30d

Description

I'm seeing failures for plotly with dev rlang:

  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Complete output:
  > library("testthat")
  > library("plotly")
  Loading required package: ggplot2
  
  Attaching package: 'plotly'
  
  The following object is masked from 'package:ggplot2':
...
  Expected `legend_title` to match regexp "factor\\(cyl\\)$".
  Actual text:
  ✖ │ factor(cyl)<br />factor(vs)
  
  [ FAIL 2 | WARN 23 | SKIP 63 | PASS 1449 ]
  Deleting unused snapshots: 'ggplot-contour/contour.svg' and
  'ggplot-heatmap/heatmap.svg'
  Error:
  ! Test failures.
  Execution halted

Claude traced this to rlang::hash() producing different hashes in the new version (which was deliberate to fix other stability issues see https://github.com/r-lib/rlang/issues/1681#issuecomment-4267291605):

The chain is:

  1. rlang dev changed hash() values (commit 229db8154 — "Stable hash()"). The NEWS says: "with this version all hash values will now be different". This is intentional for cross-R-version
  stability.
  2. ggplot2's Guides$merge sorts by hash. In Guides$merge(), each guide gets a key {order}_{hash} and then split(pairs, hashes) is called. R's split() returns groups in alphabetical order
  of factor levels. So the final guide order depends on the alphabetical sort of the hash strings.
  3. plotly concatenates guide titles in order. At ggplotly.R:1071-1072:
  legendTitles <- compact(lapply(gdefs, function(g) if (inherits(g, "legend")) g$title else NULL))
  legendTitle <- paste(legendTitles, collapse = br())
  4. The plotly test asserts a specific order (test-ggplot-legend.R:32-33):
  expect_match(legend_title, "^factor\\(vs\\)")   # vs first
  expect_match(legend_title, "factor\\(cyl\\)$")  # cyl last

  With CRAN rlang, the hashes sort as 99_124c... (vs) < 99_4d3... (cyl) → title = factor(vs)<br />factor(cyl).
  With dev rlang, the hashes sort as 99_aefe... (cyl) < 99_ebf7... (vs) → title = factor(cyl)<br />factor(vs).

I've talked to the ggplot2 maintainers and they recommend using the order argument to make guide placement deterministic. Claude also suggests this fix (unverified):

diff --git a/tests/testthat/test-ggplot-legend.R b/tests/testthat/test-ggplot-legend.R
index 9dd92ea..d1d97e5 100644
--- a/tests/testthat/test-ggplot-legend.R
+++ b/tests/testthat/test-ggplot-legend.R
@@ -29,8 +29,8 @@ test_that("Discrete colour and shape get merged into one legend", {
     nms, paste0("(", d$vs, ",", d$cyl, ")")
   )
   legend_title <- info$layout$legend$title$text
-  expect_match(legend_title, "^factor\\(vs\\)")
-  expect_match(legend_title, "factor\\(cyl\\)$")
+  expect_match(legend_title, "factor\\(vs\\)")
+  expect_match(legend_title, "factor\\(cyl\\)")
 })

As a heads up, I'm planning to release this rlang update within the next two weeks.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

The failure is covered by tests/testthat/test-ggplot-legend.R, and guide titles are assembled in ggplotly.R around lines 1071-1072. Start by running the failing test with the dev rlang version and inspect how guide order is determined. Done means the legend-title test passes with deterministic ordering under the updated rlang.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
data-visualization, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.