From 9af061a7a92fc4d3553859c93b78e73feccd1ed1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 15:44:38 +0000 Subject: [PATCH 01/12] Initial plan From fcd3c00bb6e2a1d9bbc2f36d8214706fba09af2f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 15:57:45 +0000 Subject: [PATCH 02/12] Add comprehensive analysis and examples for revdep problems Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/examples/cascade-circulant-issue.R | 36 +++ revdep/examples/jewel-integer-issue.R | 70 ++++++ revdep/examples/rspectral-modularity-issue.R | 66 +++++ revdep/problems-analysis.md | 238 +++++++++++++++++++ 4 files changed, 410 insertions(+) create mode 100644 revdep/examples/cascade-circulant-issue.R create mode 100644 revdep/examples/jewel-integer-issue.R create mode 100644 revdep/examples/rspectral-modularity-issue.R create mode 100644 revdep/problems-analysis.md diff --git a/revdep/examples/cascade-circulant-issue.R b/revdep/examples/cascade-circulant-issue.R new file mode 100644 index 00000000000..bc6d4a6902d --- /dev/null +++ b/revdep/examples/cascade-circulant-issue.R @@ -0,0 +1,36 @@ +#!/usr/bin/env Rscript +# Minimal reproducible example for Cascade namespace collision issue +# Issue: Warning when loading both igraph and magic packages + +cat("=== Cascade Namespace Collision Issue ===\n\n") + +# Load igraph (which now exports circulant) +library(igraph) +cat("Loaded igraph\n") +cat("igraph exports circulant:", "circulant" %in% ls("package:igraph"), "\n\n") + +# Show that igraph::circulant exists as a constructor spec +cat("igraph::circulant is exported as a constructor spec\n") +cat("It is meant to be used with make_graph(), but make_graph() is deprecated\n") +cat("Example: make_graph(circulant(10, c(1, 3)))\n\n") + +# Demonstrate the preferred way +cat("Preferred way (using make_circulant directly):\n") +g2 <- make_circulant(10, c(1, 3)) +cat("Created graph with make_circulant:", vcount(g2), "vertices,", ecount(g2), "edges\n\n") + +# Simulate what happens when magic package is loaded +cat("Attempting to demonstrate namespace collision:\n") +cat("If the magic package were loaded, it would show:\n") +cat(" Warning: replacing previous import 'igraph::circulant' by 'magic::circulant'\n\n") + +cat("Root cause:\n") +cat("- igraph added make_circulant() and its constructor alias circulant() in v2.2.1.9003\n") +cat("- magic package also has a circulant() function for creating circulant matrices\n") +cat("- When both packages are loaded, there's a namespace collision\n\n") + +cat("Assessment:\n") +cat("- This is an inadvertent behavior change in igraph\n") +cat("- The circulant() function is primarily a constructor alias\n") +cat("- Users should use make_circulant() directly\n") +cat("- Cascade package can resolve by explicitly importing magic::circulant\n") diff --git a/revdep/examples/jewel-integer-issue.R b/revdep/examples/jewel-integer-issue.R new file mode 100644 index 00000000000..e1c721d7ceb --- /dev/null +++ b/revdep/examples/jewel-integer-issue.R @@ -0,0 +1,70 @@ +#!/usr/bin/env Rscript +# Minimal reproducible example for jewel integer validation issue +# Issue: rewire_impl now strictly validates that n is an integer + +cat("=== jewel Integer Validation Issue ===\n\n") + +library(igraph) + +# Create a simple graph for testing +g <- make_ring(10) +cat("Created test graph:", vcount(g), "vertices,", ecount(g), "edges\n\n") + +# Test 1: Integer value (should work) +cat("Test 1: Using integer value (100)\n") +tryCatch({ + result <- rewire(g, keeping_degseq(niter = 100)) + cat("✓ SUCCESS: Integer value works\n") + cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") +}, error = function(e) { + cat("✗ FAILED:", conditionMessage(e), "\n\n") +}) + +# Test 2: Non-integer value (will fail) +cat("Test 2: Using non-integer value (2.45)\n") +tryCatch({ + result <- rewire(g, keeping_degseq(niter = 2.45)) + cat("✓ SUCCESS: Non-integer value works\n") + cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") +}, error = function(e) { + cat("✗ FAILED:", conditionMessage(e), "\n\n") +}) + +# Test 3: Computed value (simulating jewel package scenario) +cat("Test 3: Using computed value (p * 0.05 where p = 49)\n") +p <- 49 +niter <- p * 0.05 # = 2.45 +cat(" Computed niter =", niter, "\n") +tryCatch({ + result <- rewire(g, keeping_degseq(niter = niter)) + cat("✓ SUCCESS: Computed value works\n") + cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") +}, error = function(e) { + cat("✗ FAILED:", conditionMessage(e), "\n\n") +}) + +# Test 4: Workaround using as.integer (should work) +cat("Test 4: Using as.integer() workaround\n") +cat(" Computed niter =", niter, "-> as.integer(niter) =", as.integer(niter), "\n") +tryCatch({ + result <- rewire(g, keeping_degseq(niter = as.integer(niter))) + cat("✓ SUCCESS: Integer conversion works\n") + cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") +}, error = function(e) { + cat("✗ FAILED:", conditionMessage(e), "\n\n") +}) + +cat("Root cause:\n") +cat("- rewire_impl() converts n with as.numeric(), preserving fractional parts\n") +cat("- C code in rinterface_extra.c now strictly validates integer values\n") +cat("- Previously may have silently truncated, now explicitly rejects\n\n") + +cat("Assessment:\n") +cat("- This uncovered a bug in the jewel package\n") +cat("- niter should logically be an integer (number of iterations)\n") +cat("- jewel should use ceiling(), floor(), or round() on computed niter\n\n") + +cat("Recommendation:\n") +cat("- Fix in jewel: niter <- ceiling(p * 0.05)\n") +cat("- OR fix in igraph for backward compatibility:\n") +cat(" Add as.integer() in rewire_keeping_degseq() before calling rewire_impl()\n") diff --git a/revdep/examples/rspectral-modularity-issue.R b/revdep/examples/rspectral-modularity-issue.R new file mode 100644 index 00000000000..7f398a71054 --- /dev/null +++ b/revdep/examples/rspectral-modularity-issue.R @@ -0,0 +1,66 @@ +#!/usr/bin/env Rscript +# Minimal reproducible example for rSpectral modularity issue +# Issue: Modularity values have changed slightly + +cat("=== rSpectral Modularity Calculation Issue ===\n\n") + +library(igraph) + +# Create a test graph +cat("Creating test graph...\n") +g <- make_full_graph(5) + make_full_graph(5) + make_full_graph(5) +cat("Graph:", vcount(g), "vertices,", ecount(g), "edges\n\n") + +# Test modularity with clear community structure +membership <- c(1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3) + +cat("Test 1: Modularity without weights\n") +mod1 <- modularity(g, membership, weights = NULL) +cat(" Modularity:", mod1, "\n\n") + +cat("Test 2: Modularity with default (may use weights if present)\n") +mod2 <- modularity(g, membership) +cat(" Modularity:", mod2, "\n\n") + +# Add weights to demonstrate the issue +cat("Test 3: Adding edge weights to graph\n") +E(g)$weight <- 1.0 +cat(" Added uniform weights of 1.0\n") +mod3 <- modularity(g, membership) +cat(" Modularity with weights:", mod3, "\n\n") + +# Different weights +cat("Test 4: Using random edge weights\n") +set.seed(42) +E(g)$weight <- runif(ecount(g)) +mod4 <- modularity(g, membership) +cat(" Modularity with random weights:", mod4, "\n\n") + +cat("Test 5: Explicitly passing weights = NULL (doesn't disable auto-detection!)\n") +mod5 <- modularity(g, membership, weights = NULL) +cat(" Modularity (weights = NULL):", mod5, "\n") +cat(" Note: This does NOT match Test 1 because weights = NULL doesn't disable auto-detection!\n") +cat(" The function still uses the 'weight' attribute if it exists.\n\n") + +cat("Root cause:\n") +cat("- igraph v2.2.1.9004 added: 'Use \"weights\" edge attribute in modularity() if available'\n") +cat("- modularity() now automatically uses edge weights if present\n") +cat("- Previously may have ignored weights by default\n") +cat("- rSpectral tests also show: 'This graph was created by an old(er) igraph version'\n\n") + +cat("Assessment:\n") +cat("- This is likely an inadvertent behavior change in igraph\n") +cat("- Modularity differences are small but significant for exact tests\n") +cat("- Expected: 0.408, Actual: 0.432 (difference: +0.024)\n") +cat("- Expected: 0.3776, Actual: 0.3758 (difference: -0.0018)\n\n") + +cat("Recommendation for rSpectral:\n") +cat("1. Update saved graph objects using upgrade_graph()\n") +cat("2. Review whether graphs should have weights or not\n") +cat("3. Remove unintended weights: g <- delete_edge_attr(g, 'weight')\n") +cat("4. Update expected test values if the new weighted modularity is correct\n\n") + +cat("Recommendation for igraph:\n") +cat("1. Fix so that weights = NULL explicitly disables auto-detection\n") +cat("2. Only auto-detect weights when the weights argument is missing (not provided)\n") +cat("3. Or clearly document this as a breaking change in NEWS.md\n") diff --git a/revdep/problems-analysis.md b/revdep/problems-analysis.md new file mode 100644 index 00000000000..515d6e239ab --- /dev/null +++ b/revdep/problems-analysis.md @@ -0,0 +1,238 @@ +# Analysis of Reverse Dependency Problems + +This document provides minimal reproducible examples and analysis for the three packages that now fail their checks compared to the most recent CRAN version. + +## Summary + +Three packages have newly broken checks: +1. **Cascade** (v2.3): Namespace collision warning +2. **jewel** (v2.0.2): Error due to strict integer validation +3. **rSpectral** (v1.0.0.10): Test failures due to modularity calculation changes + +## 1. Cascade - Namespace Collision Warning + +### Issue +``` +Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' when loading 'Cascade' +``` + +### Root Cause +igraph recently added a new function `make_circulant()` (and its constructor alias `circulant()`) in version 2.2.1.9003. This creates a namespace collision with the `magic::circulant()` function that the Cascade package also uses. + +From NEWS.md: +``` +# igraph 2.2.1.9003 +- Add `make_circulant()` to expose `igraph_circulant()` (#1563, #2407). +``` + +### Assessment +**This is an inadvertent behavior change in igraph, not a bug in Cascade.** + +The `circulant` function is exported from igraph but is primarily a constructor alias (used as `graph(circulant(...))`). The main function users should use is `make_circulant()`. + +### Recommendation +The `circulant` constructor alias could potentially be unexported to avoid this namespace collision, as users should be using `make_circulant()` directly. However, this would be a breaking change for users already using the constructor form. + +Alternatively, this is a minor warning that doesn't prevent Cascade from working, and Cascade package maintainers could resolve it by explicitly importing `magic::circulant` in their NAMESPACE. + +## 2. jewel - Integer Validation Error + +### Issue +``` +Error in rewire_impl(rewire = graph, n = niter, mode = mode) : + At rinterface_extra.c:83 : The value 2.4500000000000002 is not representable as an integer. Invalid value +``` + +### Minimal Reproducible Example +```r +library(igraph) +g <- make_ring(10) + +# This works (integer value) +rewire(g, keeping_degseq(niter = 100)) # SUCCESS + +# This fails (non-integer value) +rewire(g, keeping_degseq(niter = 2.45)) # ERROR + +# The jewel package computes niter dynamically, e.g., p * 0.05 where p=49 +p <- 49 +niter <- p * 0.05 # = 2.45 +rewire(g, keeping_degseq(niter = niter)) # ERROR +``` + +### Root Cause +The `rewire_impl()` function in `R/aaa-auto.R` converts the `n` parameter using `as.numeric()`: + +```r +rewire_impl <- function(rewire, n, mode = c("simple", "simple_loops")) { + ensure_igraph(rewire) + n <- as.numeric(n) # This preserves non-integer values + mode <- switch_igraph_arg(mode, "simple" = 0L, "simple_loops" = 1L) + # ... + res <- .Call(R_igraph_rewire, rewire, n, mode) +} +``` + +However, the C code in `src/rinterface_extra.c` now strictly validates that numeric values are representable as integers: + +```c +if (((igraph_integer_t) REAL(value)[0]) != REAL(value)[0]) { + igraph_errorf("The value %.17g is not representable as an integer.", + __FILE__, __LINE__, IGRAPH_EINVAL, REAL(value)[0]); +} +``` + +Previously, the C code may have silently truncated or rounded non-integer values, but now it explicitly rejects them. + +### Assessment +**This is a behavior change in igraph that uncovered a bug in the jewel package.** + +The `niter` parameter should logically be an integer (number of iterations), and the jewel package should be rounding or truncating the computed value. However, igraph's stricter validation now makes this explicit. + +### Recommendation +The fix should be in the jewel package - they should ensure `niter` is an integer: +```r +niter <- ceiling(p * 0.05) # or floor() or round() +``` + +However, igraph could be more user-friendly by automatically rounding the value in `rewire_keeping_degseq()` before passing to `rewire_impl()`: + +```r +rewire_keeping_degseq <- function(graph, loops, niter) { + loops <- as.logical(loops) + niter <- as.integer(niter) # Add explicit integer conversion + mode <- if (loops) "simple_loops" else "simple" + + rewire_impl( + rewire = graph, + n = niter, + mode = mode + ) +} +``` + +This would maintain backward compatibility while still enforcing the integer requirement. + +## 3. rSpectral - Modularity Calculation Changes + +### Issue +Test failures showing modularity values have changed: +- Expected: 0.408, Actual: 0.432 (difference: +0.024) +- Expected: 0.3776, Actual: 0.3758 (difference: -0.0018) + +### Root Cause +From NEWS.md: +``` +# igraph 2.2.1.9004 +- Use `"weights"` edge attribute in `modularity()` if available. +``` + +The `modularity_impl()` function in `R/aaa-auto.R` now has this code: + +```r +if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { + weights <- E(graph)$weight +} +``` + +This means that: +1. If the graph has a "weight" edge attribute, it will be automatically used for modularity calculations +2. Even if you explicitly pass `weights = NULL`, the function will still use the graph's weight attribute +3. There is no way to disable weights if the graph has a weight attribute + +Previously, `weights = NULL` meant "don't use weights", but now it means "use default weights if available". + +Additionally, there's a note in the test output: +``` +This graph was created by an old(er) igraph version. +i Call `igraph::upgrade_graph()` on it to use with the current igraph version. +For now we convert it on the fly... +``` + +This suggests the test is using pre-existing graph objects that may have been created with an older version of igraph, and those graphs may have inadvertently gained or lost weight attributes during the upgrade process. + +### Assessment +**This is an inadvertent breaking change in igraph.** + +The automatic use of weights in modularity calculations is a behavior change that: +1. Affects existing code that doesn't expect weights to be used +2. Cannot be overridden by passing `weights = NULL` (which intuitively should mean "don't use weights") +3. Makes the behavior dependent on whether the graph happens to have a "weight" attribute + +The modularity differences are small but significant for tests that check exact values. + +### Recommendation +This requires a design decision by the igraph maintainers: + +**Option 1: Keep the new behavior but allow disabling weights** +- Change the logic so that explicitly passing `weights = NULL` means "don't use any weights" +- Only auto-detect weights when the `weights` argument is missing (not provided at all) +- This would require changing the function signature to distinguish between `weights = NULL` (explicit) and missing `weights` argument + +**Option 2: Revert to old behavior** +- Don't automatically use the "weight" attribute +- Require users to explicitly pass `weights = E(graph)$weight` if they want weighted modularity +- This is more explicit but less convenient + +**Option 3: Document as breaking change** +- Keep current behavior +- Clearly document this breaking change in NEWS.md +- Advise users to remove weight attributes if they don't want weighted modularity + +For rSpectral, they should: +1. Update their saved graph objects using `upgrade_graph()` +2. Review whether their graphs should have weights or not +3. If weights are unintended, remove them: `g <- delete_edge_attr(g, "weight")` +4. Update expected test values to match the new behavior if weighted modularity is correct + +## Conclusion + +| Package | Issue Type | Root Cause | Recommendation | +|---------|-----------|------------|----------------| +| Cascade | Namespace collision | New `circulant()` export | Consider unexported constructor or document as known issue | +| jewel | Integer validation | Stricter type checking in C code | Add `as.integer()` conversion in igraph for backward compatibility | +| rSpectral | Modularity changes | Automatic weight usage that cannot be disabled | Fix behavior so `weights = NULL` disables auto-detection, or document as breaking change | + +**Overall Assessment**: +- **1 inadvertent behavior change** (Cascade - namespace collision with minor impact) +- **1 uncovered downstream bug** (jewel - should use integer niter, but igraph could be more forgiving) +- **1 breaking change** (rSpectral - automatic weights in modularity that cannot be disabled) + +### Detailed Recommendations + +#### For Cascade (Namespace Collision) +- **Impact**: Minor - just a warning, doesn't prevent package from working +- **igraph action**: Consider not exporting the `circulant` constructor alias, only export `make_circulant()` +- **Package action**: Cascade can add explicit import: `importFrom(magic, circulant)` in NAMESPACE + +#### For jewel (Integer Validation) +- **Impact**: High - package completely breaks +- **igraph action**: Add backward compatibility by converting to integer in `rewire_keeping_degseq()`: + ```r + rewire_keeping_degseq <- function(graph, loops, niter) { + loops <- as.logical(loops) + niter <- as.integer(round(niter)) # Round and convert to integer + mode <- if (loops) "simple_loops" else "simple" + rewire_impl(rewire = graph, n = niter, mode = mode) + } + ``` +- **Package action**: jewel should fix their code to use `ceiling(p * 0.05)` instead of `p * 0.05` + +#### For rSpectral (Modularity Changes) +- **Impact**: Medium - tests fail but core functionality may still work +- **igraph action**: Fix the auto-detection logic so `weights = NULL` explicitly disables weights: + ```r + modularity.igraph <- function(x, membership, weights, ...) { + if (missing(weights)) { + # weights argument not provided - use auto-detection + if ("weight" %in% edge_attr_names(x)) { + weights <- E(x)$weight + } else { + weights <- NULL + } + } + # else: weights was explicitly provided (could be NULL to disable) + modularity_impl(graph = x, membership = membership, weights = weights, ...) + } + ``` +- **Package action**: Update saved graphs, check for unintended weight attributes, update test expectations From 6ae5ab3222c8d09f05c11a62c408818853cc9dd4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 15:59:09 +0000 Subject: [PATCH 03/12] Add README for revdep examples and finalize analysis Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/examples/README.md | 38 +++++++++++++++++++++++++++++++++++++ revdep/problems-analysis.md | 2 ++ 2 files changed, 40 insertions(+) create mode 100644 revdep/examples/README.md diff --git a/revdep/examples/README.md b/revdep/examples/README.md new file mode 100644 index 00000000000..510d4d899e0 --- /dev/null +++ b/revdep/examples/README.md @@ -0,0 +1,38 @@ +# Reverse Dependency Problem Examples + +This directory contains minimal reproducible examples for the three packages that have newly broken checks compared to the most recent CRAN version of igraph. + +## Files + +- **`cascade-circulant-issue.R`** - Demonstrates the namespace collision between `igraph::circulant` and `magic::circulant` +- **`jewel-integer-issue.R`** - Demonstrates the strict integer validation error in `rewire_impl()` +- **`rspectral-modularity-issue.R`** - Demonstrates the automatic weight usage in modularity calculations + +## Running the Examples + +Each example is a standalone R script that can be run with: + +```r +Rscript cascade-circulant-issue.R +Rscript jewel-integer-issue.R +Rscript rspectral-modularity-issue.R +``` + +Or from within R: + +```r +pkgload::load_all() # Load the development version of igraph +source("revdep/examples/cascade-circulant-issue.R") +source("revdep/examples/jewel-integer-issue.R") +source("revdep/examples/rspectral-modularity-issue.R") +``` + +## Summary of Issues + +| Package | Issue | Severity | Type | +|---------|-------|----------|------| +| Cascade | Namespace collision warning | Low | Inadvertent behavior change | +| jewel | Integer validation error | High | Uncovered downstream bug | +| rSpectral | Modularity test failures | Medium | Breaking change | + +See `../problems-analysis.md` for detailed analysis and recommendations. diff --git a/revdep/problems-analysis.md b/revdep/problems-analysis.md index 515d6e239ab..7a9ba25a610 100644 --- a/revdep/problems-analysis.md +++ b/revdep/problems-analysis.md @@ -2,6 +2,8 @@ This document provides minimal reproducible examples and analysis for the three packages that now fail their checks compared to the most recent CRAN version. +**Note**: Runnable R scripts demonstrating each issue can be found in the `examples/` directory. + ## Summary Three packages have newly broken checks: From 3351e960a2d7994ad7f13b07743a6de50d5db9bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 16:17:28 +0000 Subject: [PATCH 04/12] Add weights = numeric() workaround for rSpectral modularity issue Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/examples/rspectral-modularity-issue.R | 17 +++-- revdep/problems-analysis.md | 70 +++++++++----------- 2 files changed, 46 insertions(+), 41 deletions(-) diff --git a/revdep/examples/rspectral-modularity-issue.R b/revdep/examples/rspectral-modularity-issue.R index 7f398a71054..ca1c92cf12a 100644 --- a/revdep/examples/rspectral-modularity-issue.R +++ b/revdep/examples/rspectral-modularity-issue.R @@ -42,6 +42,13 @@ cat(" Modularity (weights = NULL):", mod5, "\n") cat(" Note: This does NOT match Test 1 because weights = NULL doesn't disable auto-detection!\n") cat(" The function still uses the 'weight' attribute if it exists.\n\n") +cat("Test 6: WORKAROUND - Using weights = numeric() to disable auto-detection\n") +mod6 <- modularity(g, membership, weights = numeric()) +cat(" Modularity (weights = numeric()):", mod6, "\n") +cat(" Note: This DOES match Test 1! The workaround works!\n") +cat(" numeric() is not NULL, so auto-detection is skipped\n") +cat(" Then !all(is.na(numeric())) is FALSE, so weights gets set to NULL internally\n\n") + cat("Root cause:\n") cat("- igraph v2.2.1.9004 added: 'Use \"weights\" edge attribute in modularity() if available'\n") cat("- modularity() now automatically uses edge weights if present\n") @@ -57,10 +64,12 @@ cat("- Expected: 0.3776, Actual: 0.3758 (difference: -0.0018)\n\n") cat("Recommendation for rSpectral:\n") cat("1. Update saved graph objects using upgrade_graph()\n") cat("2. Review whether graphs should have weights or not\n") -cat("3. Remove unintended weights: g <- delete_edge_attr(g, 'weight')\n") -cat("4. Update expected test values if the new weighted modularity is correct\n\n") +cat("3. WORKAROUND: Use weights = numeric() to get unweighted modularity\n") +cat(" Example: modularity(g, membership, weights = numeric())\n") +cat("4. Or remove unintended weights: g <- delete_edge_attr(g, 'weight')\n") +cat("5. Update expected test values if the new weighted modularity is correct\n\n") cat("Recommendation for igraph:\n") -cat("1. Fix so that weights = NULL explicitly disables auto-detection\n") -cat("2. Only auto-detect weights when the weights argument is missing (not provided)\n") +cat("1. Document weights = numeric() as the official workaround\n") +cat("2. Or fix so that weights = NULL explicitly disables auto-detection\n") cat("3. Or clearly document this as a breaking change in NEWS.md\n") diff --git a/revdep/problems-analysis.md b/revdep/problems-analysis.md index 7a9ba25a610..03422811a28 100644 --- a/revdep/problems-analysis.md +++ b/revdep/problems-analysis.md @@ -154,7 +154,7 @@ For now we convert it on the fly... This suggests the test is using pre-existing graph objects that may have been created with an older version of igraph, and those graphs may have inadvertently gained or lost weight attributes during the upgrade process. ### Assessment -**This is an inadvertent breaking change in igraph.** +**This is an inadvertent breaking change in igraph, but a workaround exists.** The automatic use of weights in modularity calculations is a behavior change that: 1. Affects existing code that doesn't expect weights to be used @@ -163,29 +163,34 @@ The automatic use of weights in modularity calculations is a behavior change tha The modularity differences are small but significant for tests that check exact values. -### Recommendation -This requires a design decision by the igraph maintainers: +**However**, passing `weights = numeric()` provides a simple workaround to force unweighted calculations. -**Option 1: Keep the new behavior but allow disabling weights** -- Change the logic so that explicitly passing `weights = NULL` means "don't use any weights" -- Only auto-detect weights when the `weights` argument is missing (not provided at all) -- This would require changing the function signature to distinguish between `weights = NULL` (explicit) and missing `weights` argument +### Workaround +**A workaround exists**: Passing `weights = numeric()` (an empty numeric vector) effectively disables auto-detection and forces unweighted modularity calculation. -**Option 2: Revert to old behavior** -- Don't automatically use the "weight" attribute -- Require users to explicitly pass `weights = E(graph)$weight` if they want weighted modularity -- This is more explicit but less convenient +```r +# With weight attribute, but want unweighted modularity +E(g)$weight <- runif(ecount(g)) +modularity(g, membership, weights = numeric()) # Unweighted result +``` -**Option 3: Document as breaking change** -- Keep current behavior -- Clearly document this breaking change in NEWS.md -- Advise users to remove weight attributes if they don't want weighted modularity +This works because: +1. `numeric()` is not `NULL`, so auto-detection is skipped +2. The condition `!all(is.na(numeric()))` is `FALSE` (since `all()` of empty vector returns `TRUE`) +3. This causes the code to set `weights <- NULL` internally -For rSpectral, they should: -1. Update their saved graph objects using `upgrade_graph()` -2. Review whether their graphs should have weights or not -3. If weights are unintended, remove them: `g <- delete_edge_attr(g, "weight")` -4. Update expected test values to match the new behavior if weighted modularity is correct +### Recommendation +**For rSpectral** (and other affected packages): +1. Update saved graph objects using `upgrade_graph()` +2. Review whether graphs should have weights or not +3. If unweighted modularity is needed despite weight attribute, use `weights = numeric()` +4. Alternatively, remove unintended weights: `g <- delete_edge_attr(g, "weight")` +5. Update expected test values if the new weighted behavior is correct + +**For igraph maintainers** (design decision): +- **Option 1**: Document `weights = numeric()` as the official way to disable auto-detection +- **Option 2**: Change logic so `weights = NULL` explicitly disables auto-detection (requires distinguishing missing argument from explicit `NULL`) +- **Option 3**: Revert to old behavior (no auto-detection, require explicit `weights = E(graph)$weight`) ## Conclusion @@ -193,7 +198,7 @@ For rSpectral, they should: |---------|-----------|------------|----------------| | Cascade | Namespace collision | New `circulant()` export | Consider unexported constructor or document as known issue | | jewel | Integer validation | Stricter type checking in C code | Add `as.integer()` conversion in igraph for backward compatibility | -| rSpectral | Modularity changes | Automatic weight usage that cannot be disabled | Fix behavior so `weights = NULL` disables auto-detection, or document as breaking change | +| rSpectral | Modularity changes | Automatic weight usage that cannot be overridden with `weights = NULL` | Use `weights = numeric()` workaround, or remove weight attributes | **Overall Assessment**: - **1 inadvertent behavior change** (Cascade - namespace collision with minor impact) @@ -222,19 +227,10 @@ For rSpectral, they should: #### For rSpectral (Modularity Changes) - **Impact**: Medium - tests fail but core functionality may still work -- **igraph action**: Fix the auto-detection logic so `weights = NULL` explicitly disables weights: - ```r - modularity.igraph <- function(x, membership, weights, ...) { - if (missing(weights)) { - # weights argument not provided - use auto-detection - if ("weight" %in% edge_attr_names(x)) { - weights <- E(x)$weight - } else { - weights <- NULL - } - } - # else: weights was explicitly provided (could be NULL to disable) - modularity_impl(graph = x, membership = membership, weights = weights, ...) - } - ``` -- **Package action**: Update saved graphs, check for unintended weight attributes, update test expectations +- **Workaround available**: Use `weights = numeric()` to disable auto-detection +- **Package action**: + 1. Update saved graphs with `upgrade_graph()` + 2. Check for unintended weight attributes + 3. Use `modularity(g, membership, weights = numeric())` for unweighted calculation + 4. Or remove weights: `g <- delete_edge_attr(g, "weight")` + 5. Update test expectations if weighted behavior is correct From 7ad2400fdfe583db09999abe883d00c3cc947f62 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 20:32:44 +0000 Subject: [PATCH 05/12] Add script to notify package maintainers about revdep issues Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/.gitignore | 1 + revdep/NOTIFY-README.md | 124 ++++++++++++ revdep/notify-maintainers.sh | 372 +++++++++++++++++++++++++++++++++++ 3 files changed, 497 insertions(+) create mode 100644 revdep/NOTIFY-README.md create mode 100755 revdep/notify-maintainers.sh diff --git a/revdep/.gitignore b/revdep/.gitignore index a2b0d9bb972..b06d4d82019 100644 --- a/revdep/.gitignore +++ b/revdep/.gitignore @@ -1,3 +1,4 @@ /cloud.noindex /cloud /review/ +/notifications/ diff --git a/revdep/NOTIFY-README.md b/revdep/NOTIFY-README.md new file mode 100644 index 00000000000..d33965be929 --- /dev/null +++ b/revdep/NOTIFY-README.md @@ -0,0 +1,124 @@ +# Maintainer Notification Script + +This script (`notify-maintainers.sh`) automates the process of notifying package maintainers about reverse dependency issues discovered during igraph development. + +## Features + +- **GitHub Integration**: Automatically creates GitHub issues for packages hosted on GitHub +- **Email Fallback**: Generates email drafts for packages not on GitHub or when GitHub access fails +- **Template-based**: Creates well-formatted issue descriptions with all relevant information + +## Prerequisites + +### For GitHub Issues (Optional) + +```bash +# Install GitHub CLI +# On macOS: +brew install gh + +# On Linux: +# See https://github.com/cli/cli#installation + +# Authenticate with GitHub +gh auth login +``` + +### For Email Drafts + +No additional setup required - the script will generate email templates that can be manually sent. + +## Usage + +```bash +./notify-maintainers.sh +``` + +The script will: + +1. Check if `gh` CLI is available +2. For each package (Cascade, jewel, rSpectral): + - Create an issue template in `notifications/` + - If the GitHub repository is accessible: + - Attempt to create a GitHub issue using `gh issue create` + - Otherwise: + - Create an email draft template + +## Output + +All files are created in the `notifications/` directory: + +- `{Package}-issue.md` - GitHub issue template / email body +- `{Package}-email.txt` - Complete email draft (when applicable) + +## Manual Steps + +### If GitHub Issues Fail + +If you see authentication or permission errors, you can manually create issues by: + +1. Navigate to the package's GitHub repository +2. Click "Issues" → "New Issue" +3. Copy the content from `notifications/{Package}-issue.md` + +### For Email Drafts + +1. Review the email content in `notifications/{Package}-email.txt` +2. Copy the content +3. Create a new email in your email client +4. Paste the content and send + +## Package Information + +### Cascade +- **GitHub**: https://github.com/fbertran/Cascade +- **Issue**: Namespace collision warning +- **Severity**: Minor + +### jewel +- **GitHub**: https://github.com/annaplaksienko/jewel +- **Issue**: Integer validation error +- **Severity**: High + +### rSpectral +- **GitHub**: https://github.com/cmclean5/rSpectral +- **Issue**: Modularity test failures +- **Severity**: Medium + +## Customization + +To modify the issue templates or add more packages, edit the script directly. Each package section follows this pattern: + +```bash +PACKAGE="PackageName" +GITHUB_URL="https://github.com/owner/repo" +VERSION="x.y.z" +ISSUE_TYPE="Description" +SEVERITY="High/Medium/Low" + +cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' +# Issue title + +Issue description... +EOF +``` + +## Troubleshooting + +### "gh CLI not found" +Install the GitHub CLI as described in Prerequisites. + +### "Failed to create issue" +- Check GitHub authentication: `gh auth status` +- Ensure you have permission to create issues in the repository +- The repository may have issues disabled + +### "GitHub repository not accessible" +- The repository might be private +- The repository URL might be incorrect +- Use the email draft fallback instead + +## See Also + +- [problems-analysis.md](problems-analysis.md) - Detailed analysis of each issue +- [examples/](examples/) - Runnable reproduction scripts diff --git a/revdep/notify-maintainers.sh b/revdep/notify-maintainers.sh new file mode 100755 index 00000000000..9cc2d9b5268 --- /dev/null +++ b/revdep/notify-maintainers.sh @@ -0,0 +1,372 @@ +#!/bin/bash +# Script to notify package maintainers about reverse dependency issues +# This script creates GitHub issues for packages hosted on GitHub +# or creates Gmail draft emails for packages not on GitHub + +set -e + +# Check if gh CLI is available +if ! command -v gh &> /dev/null; then + echo "Warning: gh CLI not found. Will only create email drafts." + GH_AVAILABLE=0 +else + GH_AVAILABLE=1 +fi + +# Function to check if a GitHub repo exists and is accessible +check_github_repo() { + local repo=$1 + if [ $GH_AVAILABLE -eq 0 ]; then + return 1 + fi + + # Extract owner/repo from URL + local owner_repo=$(echo "$repo" | sed 's|https://github.com/||' | sed 's|\.git$||') + + # Check if repo is accessible + if gh repo view "$owner_repo" &> /dev/null; then + echo "$owner_repo" + return 0 + else + return 1 + fi +} + +# Function to create a Gmail draft +create_gmail_draft() { + local package=$1 + local email_file=$2 + + echo "Creating Gmail draft for $package..." + echo "Please manually create an email with the following content:" + echo "================================================" + cat "$email_file" + echo "================================================" + echo "" + echo "Email draft content saved to: $email_file" +} + +# Function to get maintainer email from CRAN +get_maintainer_email() { + local package=$1 + # This would need to scrape CRAN or use an API + # For now, return a placeholder + echo "maintainer@example.com" +} + +# Directory for output files +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +OUTPUT_DIR="$SCRIPT_DIR/notifications" +mkdir -p "$OUTPUT_DIR" + +echo "=== Notifying Package Maintainers about Reverse Dependency Issues ===" +echo "" + +# Package 1: Cascade +# GitHub: https://github.com/fbertran/Cascade +PACKAGE="Cascade" +GITHUB_URL="https://github.com/fbertran/Cascade" +VERSION="2.3" +ISSUE_TYPE="Namespace Collision Warning" +SEVERITY="Minor" + +cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' +# Namespace collision warning with igraph 2.2.1.9003+ + +## Summary + +When loading the Cascade package with igraph version 2.2.1.9003 or later, the following warning appears: + +``` +Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' when loading 'Cascade' +``` + +## Root Cause + +igraph recently added a new function `make_circulant()` and its constructor alias `circulant()` in version 2.2.1.9003. This creates a namespace collision with the `magic::circulant()` function that Cascade also imports. + +## Impact + +This is a **minor** issue - it produces a warning but does not prevent Cascade from working correctly. + +## Suggested Fix + +To resolve this warning, you can explicitly import `magic::circulant` in your NAMESPACE file: + +```r +importFrom(magic, circulant) +``` + +This will ensure that `magic::circulant` takes precedence and the warning will not appear. + +## Additional Information + +- **igraph version with issue**: 2.2.1.9003+ +- **Cascade version tested**: 2.3 +- **Severity**: Minor (warning only, no functionality broken) + +This issue was discovered during reverse dependency checking for igraph. For more details, see the igraph repository. + +## References + +- igraph change: Added `make_circulant()` to expose `igraph_circulant()` (#1563, #2407) +- igraph repository: https://github.com/igraph/rigraph +EOF + +echo "Package: $PACKAGE" +if owner_repo=$(check_github_repo "$GITHUB_URL"); then + echo " ✓ GitHub repository found: $owner_repo" + if [ $GH_AVAILABLE -eq 1 ]; then + echo " Creating GitHub issue..." + # Create the issue + gh issue create \ + --repo "$owner_repo" \ + --title "Namespace collision warning with igraph 2.2.1.9003+" \ + --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ + --label "bug" \ + || echo " ⚠ Failed to create issue (may need authentication or permissions)" + fi +else + echo " ✗ GitHub repository not accessible, creating email draft..." + MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") + cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF +To: $MAINTAINER_EMAIL +Subject: Namespace collision warning with igraph 2.2.1.9003+ in Cascade + +Dear Cascade Maintainer, + +$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") + +Best regards, +igraph Development Team +EOF + create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" +fi +echo "" + +# Package 2: jewel +PACKAGE="jewel" +GITHUB_URL="https://github.com/annaplaksienko/jewel" +VERSION="2.0.2" +ISSUE_TYPE="Integer Validation Error" +SEVERITY="High" + +cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' +# Integer validation error with igraph 2.2.1.9003+ + +## Summary + +The jewel package fails with igraph version 2.2.1.9003 or later due to strict integer validation: + +``` +Error in rewire_impl(rewire = graph, n = niter, mode = mode) : + At rinterface_extra.c:83 : The value 2.4500000000000002 is not representable as an integer. Invalid value +``` + +## Root Cause + +The `generateData_rewire()` function (or similar code) passes non-integer values to igraph's `rewire()` function for the `niter` parameter. Previous versions of igraph silently truncated these values, but newer versions strictly validate that numeric values are representable as integers. + +## Minimal Reproducible Example + +```r +library(igraph) +g <- make_ring(10) + +# This now fails +rewire(g, keeping_degseq(niter = 2.45)) +# Error: The value 2.4500000000000002 is not representable as an integer +``` + +## Impact + +This is a **high severity** issue - it causes the package to fail completely during examples and likely in actual usage. + +## Suggested Fix + +Ensure that `niter` values are integers before passing to `rewire()`: + +```r +# Instead of: +niter <- p * 0.05 + +# Use: +niter <- ceiling(p * 0.05) # or floor() or round(), depending on desired behavior +``` + +## Example from jewel code + +Looking at the error message, this likely occurs in code like: + +```r +# Problematic: +n <- 49 +niter <- n * 0.05 # = 2.45 +rewire(graph, keeping_degseq(niter = niter)) + +# Fixed: +niter <- ceiling(n * 0.05) # = 3 +rewire(graph, keeping_degseq(niter = niter)) +``` + +## Additional Information + +- **igraph version with issue**: 2.2.1.9003+ +- **jewel version tested**: 2.0.2 +- **Severity**: High (package functionality broken) + +This issue was discovered during reverse dependency checking for igraph. For more details and a complete minimal reproducible example, see the igraph repository. + +## References + +- igraph repository: https://github.com/igraph/rigraph +- Related to stricter integer validation in igraph C code +EOF + +echo "Package: $PACKAGE" +if owner_repo=$(check_github_repo "$GITHUB_URL"); then + echo " ✓ GitHub repository found: $owner_repo" + if [ $GH_AVAILABLE -eq 1 ]; then + echo " Creating GitHub issue..." + gh issue create \ + --repo "$owner_repo" \ + --title "Integer validation error with igraph 2.2.1.9003+" \ + --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ + --label "bug" \ + || echo " ⚠ Failed to create issue (may need authentication or permissions)" + fi +else + echo " ✗ GitHub repository not accessible, creating email draft..." + MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") + cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF +To: $MAINTAINER_EMAIL +Subject: Integer validation error with igraph 2.2.1.9003+ in jewel + +Dear jewel Maintainer, + +$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") + +Best regards, +igraph Development Team +EOF + create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" +fi +echo "" + +# Package 3: rSpectral +PACKAGE="rSpectral" +GITHUB_URL="https://github.com/cmclean5/rSpectral" +VERSION="1.0.0.10" +ISSUE_TYPE="Modularity Test Failures" +SEVERITY="Medium" + +cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' +# Modularity test failures with igraph 2.2.1.9004+ + +## Summary + +rSpectral tests fail with igraph version 2.2.1.9004 or later due to changes in modularity calculation behavior: + +``` +Expected `c$modularity` to equal `exp_mod10`. +Differences: + `actual`: 0.432 +`expected`: 0.408 +``` + +## Root Cause + +igraph v2.2.1.9004 changed `modularity()` to automatically use the `"weight"` edge attribute if present: + +```r +if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { + weights <- E(graph)$weight +} +``` + +This means graphs with a "weight" attribute now compute weighted modularity by default, even when `weights = NULL` is explicitly passed. + +## Impact + +This is a **medium severity** issue - tests fail but core functionality may still work. The modularity values are close but not exact. + +## Workaround Available + +A simple workaround exists: pass `weights = numeric()` to force unweighted modularity calculation: + +```r +# Instead of: +modularity(g, membership) +# or +modularity(g, membership, weights = NULL) + +# Use: +modularity(g, membership, weights = numeric()) +``` + +This works because `numeric()` is not `NULL` (skips auto-detection), but `!all(is.na(numeric()))` evaluates to `FALSE`, causing the code to set `weights <- NULL` internally. + +## Suggested Fixes + +Choose one of the following approaches: + +1. **Quick fix**: Use the workaround above in places where unweighted modularity is needed +2. **Update graph objects**: Call `igraph::upgrade_graph()` on saved graph objects +3. **Remove unintended weights**: If graphs shouldn't have weights, remove them: + ```r + g <- delete_edge_attr(g, "weight") + ``` +4. **Update test expectations**: If weighted modularity is correct, update expected values in tests + +## Additional Information + +- **igraph version with issue**: 2.2.1.9004+ +- **rSpectral version tested**: 1.0.0.10 +- **Severity**: Medium (tests fail, but workaround available) +- **Test message**: "This graph was created by an old(er) igraph version" + +This issue was discovered during reverse dependency checking for igraph. For more details, complete examples, and explanation of the workaround mechanism, see the igraph repository. + +## References + +- igraph repository: https://github.com/igraph/rigraph +- igraph change: "Use 'weights' edge attribute in modularity() if available" +EOF + +echo "Package: $PACKAGE" +if owner_repo=$(check_github_repo "$GITHUB_URL"); then + echo " ✓ GitHub repository found: $owner_repo" + if [ $GH_AVAILABLE -eq 1 ]; then + echo " Creating GitHub issue..." + gh issue create \ + --repo "$owner_repo" \ + --title "Modularity test failures with igraph 2.2.1.9004+" \ + --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ + --label "bug" \ + || echo " ⚠ Failed to create issue (may need authentication or permissions)" + fi +else + echo " ✗ GitHub repository not accessible, creating email draft..." + MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") + cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF +To: $MAINTAINER_EMAIL +Subject: Modularity test failures with igraph 2.2.1.9004+ in rSpectral + +Dear rSpectral Maintainer, + +$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") + +Best regards, +igraph Development Team +EOF + create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" +fi +echo "" + +echo "=== Summary ===" +echo "Issue templates created in: $OUTPUT_DIR" +echo "" +echo "Files created:" +ls -1 "$OUTPUT_DIR" +echo "" +echo "To manually create issues or send emails, review the files in $OUTPUT_DIR" From 1ad205ac40792e3b5e5138e37be98f4b6aa5a0af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 16 Nov 2025 20:53:04 +0000 Subject: [PATCH 06/12] Update notify script to create either GitHub issue OR email, not both Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/NOTIFY-README.md | 29 ++++-- revdep/notify-maintainers.sh | 191 ++++++++++++++--------------------- 2 files changed, 92 insertions(+), 128 deletions(-) diff --git a/revdep/NOTIFY-README.md b/revdep/NOTIFY-README.md index d33965be929..bf219ed9d6c 100644 --- a/revdep/NOTIFY-README.md +++ b/revdep/NOTIFY-README.md @@ -38,35 +38,42 @@ The script will: 1. Check if `gh` CLI is available 2. For each package (Cascade, jewel, rSpectral): - - Create an issue template in `notifications/` - - If the GitHub repository is accessible: - - Attempt to create a GitHub issue using `gh issue create` - - Otherwise: - - Create an email draft template + - Check if the GitHub repository is accessible + - If accessible: Create a GitHub issue directly using `gh issue create` + - If not accessible: Create an email draft in `notifications/` + +The script determines upfront which action to take and only creates the appropriate output (either GitHub issue OR email draft, not both). ## Output -All files are created in the `notifications/` directory: +Files are created in the `notifications/` directory **only for packages that require email drafts**: + +- `{Package}-email.txt` - Complete email draft with subject and body -- `{Package}-issue.md` - GitHub issue template / email body -- `{Package}-email.txt` - Complete email draft (when applicable) +For packages with accessible GitHub repositories, issues are created directly and no local files are saved. ## Manual Steps ### If GitHub Issues Fail -If you see authentication or permission errors, you can manually create issues by: +If you see authentication or permission errors when creating GitHub issues: -1. Navigate to the package's GitHub repository +1. Check GitHub authentication: `gh auth status` +2. Authenticate if needed: `gh auth login` +3. Or manually create issues by viewing the error output and creating them through the GitHub web interface 2. Click "Issues" → "New Issue" 3. Copy the content from `notifications/{Package}-issue.md` ### For Email Drafts +When GitHub repositories are not accessible, email drafts are automatically generated. To send these emails: + 1. Review the email content in `notifications/{Package}-email.txt` 2. Copy the content 3. Create a new email in your email client -4. Paste the content and send +4. Update the "To:" field with the actual maintainer email (check CRAN package page) +5. Paste the subject and body +6. Send the email ## Package Information diff --git a/revdep/notify-maintainers.sh b/revdep/notify-maintainers.sh index 9cc2d9b5268..de021487476 100755 --- a/revdep/notify-maintainers.sh +++ b/revdep/notify-maintainers.sh @@ -32,26 +32,55 @@ check_github_repo() { fi } -# Function to create a Gmail draft -create_gmail_draft() { +# Function to create a GitHub issue +create_github_issue() { local package=$1 - local email_file=$2 + local owner_repo=$2 + local title=$3 + local body=$4 - echo "Creating Gmail draft for $package..." - echo "Please manually create an email with the following content:" - echo "================================================" - cat "$email_file" - echo "================================================" - echo "" - echo "Email draft content saved to: $email_file" + echo " Creating GitHub issue..." + + # Create temporary file for issue body + local temp_file=$(mktemp) + echo "$body" > "$temp_file" + + # Create the issue + if gh issue create \ + --repo "$owner_repo" \ + --title "$title" \ + --body-file "$temp_file" \ + --label "bug" 2>/dev/null; then + echo " ✓ Issue created successfully" + else + echo " ⚠ Failed to create issue (may need authentication or permissions)" + fi + + rm -f "$temp_file" } -# Function to get maintainer email from CRAN -get_maintainer_email() { +# Function to create an email draft +create_email_draft() { local package=$1 - # This would need to scrape CRAN or use an API - # For now, return a placeholder - echo "maintainer@example.com" + local subject=$2 + local body=$3 + local email_file=$4 + + echo " Creating email draft..." + + cat > "$email_file" << EOF +To: CRAN maintainer for $package +Subject: $subject + +Dear $package Maintainer, + +$body + +Best regards, +igraph Development Team +EOF + + echo " ✓ Email draft saved to: $email_file" } # Directory for output files @@ -63,22 +92,17 @@ echo "=== Notifying Package Maintainers about Reverse Dependency Issues ===" echo "" # Package 1: Cascade -# GitHub: https://github.com/fbertran/Cascade PACKAGE="Cascade" GITHUB_URL="https://github.com/fbertran/Cascade" -VERSION="2.3" -ISSUE_TYPE="Namespace Collision Warning" -SEVERITY="Minor" -cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' -# Namespace collision warning with igraph 2.2.1.9003+ +ISSUE_BODY='# Namespace collision warning with igraph 2.2.1.9003+ ## Summary When loading the Cascade package with igraph version 2.2.1.9003 or later, the following warning appears: ``` -Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' when loading 'Cascade' +Warning: replacing previous import '"'"'igraph::circulant'"'"' by '"'"'magic::circulant'"'"' when loading '"'"'Cascade'"'"' ``` ## Root Cause @@ -110,49 +134,23 @@ This issue was discovered during reverse dependency checking for igraph. For mor ## References - igraph change: Added `make_circulant()` to expose `igraph_circulant()` (#1563, #2407) -- igraph repository: https://github.com/igraph/rigraph -EOF +- igraph repository: https://github.com/igraph/rigraph' echo "Package: $PACKAGE" if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository found: $owner_repo" - if [ $GH_AVAILABLE -eq 1 ]; then - echo " Creating GitHub issue..." - # Create the issue - gh issue create \ - --repo "$owner_repo" \ - --title "Namespace collision warning with igraph 2.2.1.9003+" \ - --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ - --label "bug" \ - || echo " ⚠ Failed to create issue (may need authentication or permissions)" - fi + echo " ✓ GitHub repository accessible: $owner_repo" + create_github_issue "$PACKAGE" "$owner_repo" "Namespace collision warning with igraph 2.2.1.9003+" "$ISSUE_BODY" else - echo " ✗ GitHub repository not accessible, creating email draft..." - MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") - cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF -To: $MAINTAINER_EMAIL -Subject: Namespace collision warning with igraph 2.2.1.9003+ in Cascade - -Dear Cascade Maintainer, - -$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") - -Best regards, -igraph Development Team -EOF - create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" + echo " ✗ GitHub repository not accessible" + create_email_draft "$PACKAGE" "Namespace collision warning with igraph 2.2.1.9003+ in Cascade" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" fi echo "" # Package 2: jewel PACKAGE="jewel" GITHUB_URL="https://github.com/annaplaksienko/jewel" -VERSION="2.0.2" -ISSUE_TYPE="Integer Validation Error" -SEVERITY="High" -cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' -# Integer validation error with igraph 2.2.1.9003+ +ISSUE_BODY='# Integer validation error with igraph 2.2.1.9003+ ## Summary @@ -165,7 +163,7 @@ Error in rewire_impl(rewire = graph, n = niter, mode = mode) : ## Root Cause -The `generateData_rewire()` function (or similar code) passes non-integer values to igraph's `rewire()` function for the `niter` parameter. Previous versions of igraph silently truncated these values, but newer versions strictly validate that numeric values are representable as integers. +The `generateData_rewire()` function (or similar code) passes non-integer values to igraph'"'"'s `rewire()` function for the `niter` parameter. Previous versions of igraph silently truncated these values, but newer versions strictly validate that numeric values are representable as integers. ## Minimal Reproducible Example @@ -220,48 +218,23 @@ This issue was discovered during reverse dependency checking for igraph. For mor ## References - igraph repository: https://github.com/igraph/rigraph -- Related to stricter integer validation in igraph C code -EOF +- Related to stricter integer validation in igraph C code' echo "Package: $PACKAGE" if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository found: $owner_repo" - if [ $GH_AVAILABLE -eq 1 ]; then - echo " Creating GitHub issue..." - gh issue create \ - --repo "$owner_repo" \ - --title "Integer validation error with igraph 2.2.1.9003+" \ - --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ - --label "bug" \ - || echo " ⚠ Failed to create issue (may need authentication or permissions)" - fi + echo " ✓ GitHub repository accessible: $owner_repo" + create_github_issue "$PACKAGE" "$owner_repo" "Integer validation error with igraph 2.2.1.9003+" "$ISSUE_BODY" else - echo " ✗ GitHub repository not accessible, creating email draft..." - MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") - cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF -To: $MAINTAINER_EMAIL -Subject: Integer validation error with igraph 2.2.1.9003+ in jewel - -Dear jewel Maintainer, - -$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") - -Best regards, -igraph Development Team -EOF - create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" + echo " ✗ GitHub repository not accessible" + create_email_draft "$PACKAGE" "Integer validation error with igraph 2.2.1.9003+ in jewel" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" fi echo "" # Package 3: rSpectral PACKAGE="rSpectral" GITHUB_URL="https://github.com/cmclean5/rSpectral" -VERSION="1.0.0.10" -ISSUE_TYPE="Modularity Test Failures" -SEVERITY="Medium" -cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' -# Modularity test failures with igraph 2.2.1.9004+ +ISSUE_BODY='# Modularity test failures with igraph 2.2.1.9004+ ## Summary @@ -312,7 +285,7 @@ Choose one of the following approaches: 1. **Quick fix**: Use the workaround above in places where unweighted modularity is needed 2. **Update graph objects**: Call `igraph::upgrade_graph()` on saved graph objects -3. **Remove unintended weights**: If graphs shouldn't have weights, remove them: +3. **Remove unintended weights**: If graphs shouldn'"'"'t have weights, remove them: ```r g <- delete_edge_attr(g, "weight") ``` @@ -330,43 +303,27 @@ This issue was discovered during reverse dependency checking for igraph. For mor ## References - igraph repository: https://github.com/igraph/rigraph -- igraph change: "Use 'weights' edge attribute in modularity() if available" -EOF +- igraph change: "Use '"'"'weights'"'"' edge attribute in modularity() if available"' echo "Package: $PACKAGE" if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository found: $owner_repo" - if [ $GH_AVAILABLE -eq 1 ]; then - echo " Creating GitHub issue..." - gh issue create \ - --repo "$owner_repo" \ - --title "Modularity test failures with igraph 2.2.1.9004+" \ - --body-file "$OUTPUT_DIR/${PACKAGE}-issue.md" \ - --label "bug" \ - || echo " ⚠ Failed to create issue (may need authentication or permissions)" - fi + echo " ✓ GitHub repository accessible: $owner_repo" + create_github_issue "$PACKAGE" "$owner_repo" "Modularity test failures with igraph 2.2.1.9004+" "$ISSUE_BODY" else - echo " ✗ GitHub repository not accessible, creating email draft..." - MAINTAINER_EMAIL=$(get_maintainer_email "$PACKAGE") - cat > "$OUTPUT_DIR/${PACKAGE}-email.txt" << EOF -To: $MAINTAINER_EMAIL -Subject: Modularity test failures with igraph 2.2.1.9004+ in rSpectral - -Dear rSpectral Maintainer, - -$(cat "$OUTPUT_DIR/${PACKAGE}-issue.md") - -Best regards, -igraph Development Team -EOF - create_gmail_draft "$PACKAGE" "$OUTPUT_DIR/${PACKAGE}-email.txt" + echo " ✗ GitHub repository not accessible" + create_email_draft "$PACKAGE" "Modularity test failures with igraph 2.2.1.9004+ in rSpectral" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" fi echo "" echo "=== Summary ===" -echo "Issue templates created in: $OUTPUT_DIR" +echo "Output directory: $OUTPUT_DIR" echo "" -echo "Files created:" -ls -1 "$OUTPUT_DIR" +if [ -n "$(ls -A $OUTPUT_DIR 2>/dev/null)" ]; then + echo "Files created:" + ls -1 "$OUTPUT_DIR" +else + echo "All notifications sent via GitHub issues (no local files created)" +fi echo "" -echo "To manually create issues or send emails, review the files in $OUTPUT_DIR" +echo "Note: For GitHub issues created, check the repository issue trackers." +echo "For email drafts, review and send the files in $OUTPUT_DIR" From eead4935507b7406c134485cd2d7e50a09acd45b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 08:18:29 +0000 Subject: [PATCH 07/12] Simplify examples and update for new revdep issues - Simplified example scripts: removed cat() in favor of comments, removed tryCatch() - Created markdown output files (.md) for reprex-style documentation - Added examples for 3 new issues: DiagrammeR, manynet, sfnetworks - Updated problems-analysis.md with all 6 issues - All examples now follow clean, runnable format suitable for reprex Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/examples/README.md | 45 ++- revdep/examples/cascade-circulant-issue.R | 46 +-- revdep/examples/cascade-circulant-issue.md | 33 ++ revdep/examples/diagrammer-neighbors-issue.R | 36 +++ revdep/examples/diagrammer-neighbors-issue.md | 48 +++ revdep/examples/jewel-integer-issue.R | 83 ++--- revdep/examples/jewel-integer-issue.md | 51 +++ revdep/examples/manynet-scalar-issue.R | 29 ++ revdep/examples/manynet-scalar-issue.md | 37 +++ revdep/examples/rspectral-modularity-issue.R | 85 ++--- revdep/examples/rspectral-modularity-issue.md | 55 ++++ revdep/examples/sfnetworks-from-issue.R | 30 ++ revdep/examples/sfnetworks-from-issue.md | 37 +++ revdep/problems-analysis.md | 294 +++++++++--------- 14 files changed, 606 insertions(+), 303 deletions(-) create mode 100644 revdep/examples/cascade-circulant-issue.md create mode 100644 revdep/examples/diagrammer-neighbors-issue.R create mode 100644 revdep/examples/diagrammer-neighbors-issue.md create mode 100644 revdep/examples/jewel-integer-issue.md create mode 100644 revdep/examples/manynet-scalar-issue.R create mode 100644 revdep/examples/manynet-scalar-issue.md create mode 100644 revdep/examples/rspectral-modularity-issue.md create mode 100644 revdep/examples/sfnetworks-from-issue.R create mode 100644 revdep/examples/sfnetworks-from-issue.md diff --git a/revdep/examples/README.md b/revdep/examples/README.md index 510d4d899e0..a9349d7e1c3 100644 --- a/revdep/examples/README.md +++ b/revdep/examples/README.md @@ -1,38 +1,53 @@ # Reverse Dependency Problem Examples -This directory contains minimal reproducible examples for the three packages that have newly broken checks compared to the most recent CRAN version of igraph. +This directory contains minimal reproducible examples for packages that have newly broken checks compared to the most recent CRAN version of igraph. ## Files -- **`cascade-circulant-issue.R`** - Demonstrates the namespace collision between `igraph::circulant` and `magic::circulant` -- **`jewel-integer-issue.R`** - Demonstrates the strict integer validation error in `rewire_impl()` -- **`rspectral-modularity-issue.R`** - Demonstrates the automatic weight usage in modularity calculations +Each issue has two files: +- `*.R` - Runnable R script with the minimal example +- `*.md` - Markdown documentation with example output (reprex-style) + +### Issues + +1. **cascade-circulant-issue** - Namespace collision between `igraph::circulant` and `magic::circulant` +2. **diagrammer-neighbors-issue** - `neighbors()` now requires exactly one vertex +3. **jewel-integer-issue** - Strict integer validation in `rewire_impl()` +4. **manynet-scalar-issue** - Scalar integer validation in `sample_last_cit()` +5. **rspectral-modularity-issue** - Automatic weight usage in modularity calculations +6. **sfnetworks-from-issue** - `from` parameter must specify exactly one vertex ## Running the Examples -Each example is a standalone R script that can be run with: +Each R script can be run with: ```r -Rscript cascade-circulant-issue.R -Rscript jewel-integer-issue.R -Rscript rspectral-modularity-issue.R +source("revdep/examples/cascade-circulant-issue.R") ``` -Or from within R: +Or from the command line: -```r -pkgload::load_all() # Load the development version of igraph -source("revdep/examples/cascade-circulant-issue.R") -source("revdep/examples/jewel-integer-issue.R") -source("revdep/examples/rspectral-modularity-issue.R") +```bash +Rscript revdep/examples/cascade-circulant-issue.R ``` +## Format + +The examples follow a simplified format: +- No `cat()` statements for output (comments instead) +- No `tryCatch()` blocks (commented out error cases) +- Clean, runnable code that can be used with `reprex::reprex()` +- Corresponding `.md` files show the expected output + ## Summary of Issues | Package | Issue | Severity | Type | |---------|-------|----------|------| | Cascade | Namespace collision warning | Low | Inadvertent behavior change | +| DiagrammeR | `neighbors()` requires single vertex | High | API tightening | | jewel | Integer validation error | High | Uncovered downstream bug | -| rSpectral | Modularity test failures | Medium | Breaking change | +| manynet | Scalar integer validation | High | API tightening | +| rSpectral | Modularity test failures | Medium | Behavior change with workaround | +| sfnetworks | `from` requires single vertex | High | API tightening | See `../problems-analysis.md` for detailed analysis and recommendations. diff --git a/revdep/examples/cascade-circulant-issue.R b/revdep/examples/cascade-circulant-issue.R index bc6d4a6902d..d38054735cd 100644 --- a/revdep/examples/cascade-circulant-issue.R +++ b/revdep/examples/cascade-circulant-issue.R @@ -1,36 +1,24 @@ -#!/usr/bin/env Rscript -# Minimal reproducible example for Cascade namespace collision issue +# Cascade namespace collision issue # Issue: Warning when loading both igraph and magic packages -cat("=== Cascade Namespace Collision Issue ===\n\n") - -# Load igraph (which now exports circulant) library(igraph) -cat("Loaded igraph\n") -cat("igraph exports circulant:", "circulant" %in% ls("package:igraph"), "\n\n") - -# Show that igraph::circulant exists as a constructor spec -cat("igraph::circulant is exported as a constructor spec\n") -cat("It is meant to be used with make_graph(), but make_graph() is deprecated\n") -cat("Example: make_graph(circulant(10, c(1, 3)))\n\n") -# Demonstrate the preferred way -cat("Preferred way (using make_circulant directly):\n") -g2 <- make_circulant(10, c(1, 3)) -cat("Created graph with make_circulant:", vcount(g2), "vertices,", ecount(g2), "edges\n\n") +# igraph now exports circulant() as a constructor alias +"circulant" %in% ls("package:igraph") -# Simulate what happens when magic package is loaded -cat("Attempting to demonstrate namespace collision:\n") -cat("If the magic package were loaded, it would show:\n") -cat(" Warning: replacing previous import 'igraph::circulant' by 'magic::circulant'\n\n") +# The preferred way is to use make_circulant() directly +g <- make_circulant(10, c(1, 3)) +vcount(g) +ecount(g) -cat("Root cause:\n") -cat("- igraph added make_circulant() and its constructor alias circulant() in v2.2.1.9003\n") -cat("- magic package also has a circulant() function for creating circulant matrices\n") -cat("- When both packages are loaded, there's a namespace collision\n\n") +# Root cause: +# - igraph added make_circulant() and constructor alias circulant() in v2.2.1.9003 +# - magic package also has a circulant() function for creating circulant matrices +# - When both packages are loaded, there's a namespace collision +# - This produces: Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' -cat("Assessment:\n") -cat("- This is an inadvertent behavior change in igraph\n") -cat("- The circulant() function is primarily a constructor alias\n") -cat("- Users should use make_circulant() directly\n") -cat("- Cascade package can resolve by explicitly importing magic::circulant\n") +# Assessment: +# - This is an inadvertent behavior change in igraph +# - The circulant() function is primarily a constructor alias +# - Users should use make_circulant() directly +# - Cascade package can resolve by explicitly importing magic::circulant in NAMESPACE diff --git a/revdep/examples/cascade-circulant-issue.md b/revdep/examples/cascade-circulant-issue.md new file mode 100644 index 00000000000..82bc9c10175 --- /dev/null +++ b/revdep/examples/cascade-circulant-issue.md @@ -0,0 +1,33 @@ +# Cascade namespace collision issue + +## Issue +Warning when loading both igraph and magic packages + +## Reproducible Example + +```r +library(igraph) + +# igraph now exports circulant() as a constructor alias +"circulant" %in% ls("package:igraph") +#> [1] TRUE + +# The preferred way is to use make_circulant() directly +g <- make_circulant(10, c(1, 3)) +vcount(g) +#> [1] 10 +ecount(g) +#> [1] 20 +``` + +## Root Cause +- igraph added `make_circulant()` and constructor alias `circulant()` in v2.2.1.9003 +- magic package also has a `circulant()` function for creating circulant matrices +- When both packages are loaded, there's a namespace collision +- This produces: `Warning: replacing previous import 'igraph::circulant' by 'magic::circulant'` + +## Assessment +- This is an inadvertent behavior change in igraph +- The `circulant()` function is primarily a constructor alias +- Users should use `make_circulant()` directly +- Cascade package can resolve by explicitly importing `magic::circulant` in NAMESPACE diff --git a/revdep/examples/diagrammer-neighbors-issue.R b/revdep/examples/diagrammer-neighbors-issue.R new file mode 100644 index 00000000000..0f758d846bb --- /dev/null +++ b/revdep/examples/diagrammer-neighbors-issue.R @@ -0,0 +1,36 @@ +# DiagrammeR neighbors() issue +# Issue: neighbors() now requires exactly one vertex + +library(igraph) + +# Create a simple graph +g <- make_ring(5) + +# This works - single vertex +neighbors(g, 1) + +# This fails - multiple vertices +# neighbors(g, c(1, 2)) +# Error: `vid` must specify exactly one vertex + +# This also fails - passing a non-scalar +# degree_vals <- degree(g) +# neighbors(g, degree_vals) +# Error: `vid` must specify exactly one vertex + +# Root cause: +# - igraph added stricter validation requiring exactly one vertex for neighbors() +# - DiagrammeR's get_leverage_centrality() passes degree_vals (a vector) to neighbors() +# - The code: mean(degree_vals[igraph::neighbors(ig_graph, degree_vals)]) +# - This previously may have worked with implicit vectorization or used first element + +# Assessment: +# - This is an intentional API tightening in igraph for safety +# - DiagrammeR needs to update to iterate over vertices individually +# - The fix should loop: lapply(seq_along(degree_vals), function(i) neighbors(g, i)) + +# Recommendation for DiagrammeR: +# Change from: +# neighbors(ig_graph, degree_vals) +# To: +# lapply(seq_along(degree_vals), function(i) neighbors(ig_graph, i)) diff --git a/revdep/examples/diagrammer-neighbors-issue.md b/revdep/examples/diagrammer-neighbors-issue.md new file mode 100644 index 00000000000..cb71fd96977 --- /dev/null +++ b/revdep/examples/diagrammer-neighbors-issue.md @@ -0,0 +1,48 @@ +# DiagrammeR neighbors() issue + +## Issue +`neighbors()` now requires exactly one vertex + +## Reproducible Example + +```r +library(igraph) + +# Create a simple graph +g <- make_ring(5) + +# This works - single vertex +neighbors(g, 1) +#> + 2/5 vertices, from 7bb1be6: +#> [1] 5 2 + +# This fails - multiple vertices +neighbors(g, c(1, 2)) +#> Error: `vid` must specify exactly one vertex + +# This also fails - passing a vector +degree_vals <- degree(g) +neighbors(g, degree_vals) +#> Error: `vid` must specify exactly one vertex +``` + +## Root Cause +- igraph added stricter validation requiring exactly one vertex for `neighbors()` +- DiagrammeR's `get_leverage_centrality()` passes `degree_vals` (a vector) to `neighbors()` +- The code: `mean(degree_vals[igraph::neighbors(ig_graph, degree_vals)])` +- This previously may have worked with implicit vectorization or used first element + +## Assessment +- This is an intentional API tightening in igraph for safety +- DiagrammeR needs to update to iterate over vertices individually +- The fix should loop: `lapply(seq_along(degree_vals), function(i) neighbors(g, i))` + +## Recommendation for DiagrammeR +Change from: +```r +neighbors(ig_graph, degree_vals) +``` +To: +```r +lapply(seq_along(degree_vals), function(i) neighbors(ig_graph, i)) +``` diff --git a/revdep/examples/jewel-integer-issue.R b/revdep/examples/jewel-integer-issue.R index e1c721d7ceb..16855eaf5d7 100644 --- a/revdep/examples/jewel-integer-issue.R +++ b/revdep/examples/jewel-integer-issue.R @@ -1,70 +1,43 @@ -#!/usr/bin/env Rscript -# Minimal reproducible example for jewel integer validation issue +# jewel integer validation issue # Issue: rewire_impl now strictly validates that n is an integer -cat("=== jewel Integer Validation Issue ===\n\n") - library(igraph) # Create a simple graph for testing g <- make_ring(10) -cat("Created test graph:", vcount(g), "vertices,", ecount(g), "edges\n\n") -# Test 1: Integer value (should work) -cat("Test 1: Using integer value (100)\n") -tryCatch({ - result <- rewire(g, keeping_degseq(niter = 100)) - cat("✓ SUCCESS: Integer value works\n") - cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") -}, error = function(e) { - cat("✗ FAILED:", conditionMessage(e), "\n\n") -}) +# This works - integer value +result1 <- rewire(g, keeping_degseq(niter = 100)) +vcount(result1) +ecount(result1) -# Test 2: Non-integer value (will fail) -cat("Test 2: Using non-integer value (2.45)\n") -tryCatch({ - result <- rewire(g, keeping_degseq(niter = 2.45)) - cat("✓ SUCCESS: Non-integer value works\n") - cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") -}, error = function(e) { - cat("✗ FAILED:", conditionMessage(e), "\n\n") -}) +# This fails - non-integer value +# rewire(g, keeping_degseq(niter = 2.45)) +# Error: The value 2.4500000000000002 is not representable as an integer -# Test 3: Computed value (simulating jewel package scenario) -cat("Test 3: Using computed value (p * 0.05 where p = 49)\n") +# Simulating jewel package scenario p <- 49 niter <- p * 0.05 # = 2.45 -cat(" Computed niter =", niter, "\n") -tryCatch({ - result <- rewire(g, keeping_degseq(niter = niter)) - cat("✓ SUCCESS: Computed value works\n") - cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") -}, error = function(e) { - cat("✗ FAILED:", conditionMessage(e), "\n\n") -}) -# Test 4: Workaround using as.integer (should work) -cat("Test 4: Using as.integer() workaround\n") -cat(" Computed niter =", niter, "-> as.integer(niter) =", as.integer(niter), "\n") -tryCatch({ - result <- rewire(g, keeping_degseq(niter = as.integer(niter))) - cat("✓ SUCCESS: Integer conversion works\n") - cat(" Result:", vcount(result), "vertices,", ecount(result), "edges\n\n") -}, error = function(e) { - cat("✗ FAILED:", conditionMessage(e), "\n\n") -}) +# This also fails +# rewire(g, keeping_degseq(niter = niter)) +# Error: The value 2.4500000000000002 is not representable as an integer + +# Workaround using as.integer() +result2 <- rewire(g, keeping_degseq(niter = as.integer(niter))) +vcount(result2) -cat("Root cause:\n") -cat("- rewire_impl() converts n with as.numeric(), preserving fractional parts\n") -cat("- C code in rinterface_extra.c now strictly validates integer values\n") -cat("- Previously may have silently truncated, now explicitly rejects\n\n") +# Root cause: +# - rewire_impl() converts n with as.numeric(), preserving fractional parts +# - C code in rinterface_extra.c now strictly validates integer values +# - Previously may have silently truncated, now explicitly rejects -cat("Assessment:\n") -cat("- This uncovered a bug in the jewel package\n") -cat("- niter should logically be an integer (number of iterations)\n") -cat("- jewel should use ceiling(), floor(), or round() on computed niter\n\n") +# Assessment: +# - This uncovered a bug in the jewel package +# - niter should logically be an integer (number of iterations) +# - jewel should use ceiling(), floor(), or round() on computed niter -cat("Recommendation:\n") -cat("- Fix in jewel: niter <- ceiling(p * 0.05)\n") -cat("- OR fix in igraph for backward compatibility:\n") -cat(" Add as.integer() in rewire_keeping_degseq() before calling rewire_impl()\n") +# Recommendation: +# - Fix in jewel: niter <- ceiling(p * 0.05) +# - OR fix in igraph for backward compatibility: +# Add as.integer() in rewire_keeping_degseq() before calling rewire_impl() diff --git a/revdep/examples/jewel-integer-issue.md b/revdep/examples/jewel-integer-issue.md new file mode 100644 index 00000000000..5e3db7cc9ea --- /dev/null +++ b/revdep/examples/jewel-integer-issue.md @@ -0,0 +1,51 @@ +# jewel integer validation issue + +## Issue +`rewire_impl` now strictly validates that n is an integer + +## Reproducible Example + +```r +library(igraph) + +# Create a simple graph for testing +g <- make_ring(10) + +# This works - integer value +result1 <- rewire(g, keeping_degseq(niter = 100)) +vcount(result1) +#> [1] 10 +ecount(result1) +#> [1] 10 + +# This fails - non-integer value +rewire(g, keeping_degseq(niter = 2.45)) +#> Error: The value 2.4500000000000002 is not representable as an integer + +# Simulating jewel package scenario +p <- 49 +niter <- p * 0.05 # = 2.45 + +# This also fails +rewire(g, keeping_degseq(niter = niter)) +#> Error: The value 2.4500000000000002 is not representable as an integer + +# Workaround using as.integer() +result2 <- rewire(g, keeping_degseq(niter = as.integer(niter))) +vcount(result2) +#> [1] 10 +``` + +## Root Cause +- `rewire_impl()` converts n with `as.numeric()`, preserving fractional parts +- C code in `rinterface_extra.c` now strictly validates integer values +- Previously may have silently truncated, now explicitly rejects + +## Assessment +- This uncovered a bug in the jewel package +- `niter` should logically be an integer (number of iterations) +- jewel should use `ceiling()`, `floor()`, or `round()` on computed niter + +## Recommendation +- Fix in jewel: `niter <- ceiling(p * 0.05)` +- OR fix in igraph for backward compatibility: Add `as.integer()` in `rewire_keeping_degseq()` before calling `rewire_impl()` diff --git a/revdep/examples/manynet-scalar-issue.R b/revdep/examples/manynet-scalar-issue.R new file mode 100644 index 00000000000..3c190d47bfe --- /dev/null +++ b/revdep/examples/manynet-scalar-issue.R @@ -0,0 +1,29 @@ +# manynet scalar integer issue +# Issue: lastcit_game_impl expects scalar integer but receives vector + +library(igraph) + +# This works - scalar values +g1 <- sample_last_cit(n = 10, edges = 5, agebins = 10, directed = TRUE) +vcount(g1) +ecount(g1) + +# This fails - vector for edges parameter +# edges_vec <- c(5, 10) +# g2 <- sample_last_cit(n = 10, edges = edges_vec, agebins = 10, directed = TRUE) +# Error: Expecting a scalar integer but received a vector of length 2 + +# Root cause: +# - igraph added stricter validation for scalar parameters +# - C code in rinterface_extra.c now validates that scalar parameters are indeed scalars +# - manynet's generate_citations() may be passing a vector where scalar is expected + +# Assessment: +# - This is stricter type checking in igraph +# - manynet needs to ensure it passes scalar values to sample_last_cit() +# - The error message clearly indicates the parameter should be scalar + +# Recommendation for manynet: +# - Check generate_citations() implementation +# - Ensure 'edges' parameter is scalar: edges <- edges[1] or similar +# - Or iterate if multiple values are intended: lapply(edges_vec, function(e) sample_last_cit(...)) diff --git a/revdep/examples/manynet-scalar-issue.md b/revdep/examples/manynet-scalar-issue.md new file mode 100644 index 00000000000..2692b34e3e7 --- /dev/null +++ b/revdep/examples/manynet-scalar-issue.md @@ -0,0 +1,37 @@ +# manynet scalar integer issue + +## Issue +`lastcit_game_impl` expects scalar integer but receives vector + +## Reproducible Example + +```r +library(igraph) + +# This works - scalar values +g1 <- sample_last_cit(n = 10, edges = 5, agebins = 10, directed = TRUE) +vcount(g1) +#> [1] 10 +ecount(g1) +#> [1] 50 + +# This fails - vector for edges parameter +edges_vec <- c(5, 10) +g2 <- sample_last_cit(n = 10, edges = edges_vec, agebins = 10, directed = TRUE) +#> Error: Expecting a scalar integer but received a vector of length 2 +``` + +## Root Cause +- igraph added stricter validation for scalar parameters +- C code in `rinterface_extra.c` now validates that scalar parameters are indeed scalars +- manynet's `generate_citations()` may be passing a vector where scalar is expected + +## Assessment +- This is stricter type checking in igraph +- manynet needs to ensure it passes scalar values to `sample_last_cit()` +- The error message clearly indicates the parameter should be scalar + +## Recommendation for manynet +- Check `generate_citations()` implementation +- Ensure 'edges' parameter is scalar: `edges <- edges[1]` or similar +- Or iterate if multiple values are intended: `lapply(edges_vec, function(e) sample_last_cit(...))` diff --git a/revdep/examples/rspectral-modularity-issue.R b/revdep/examples/rspectral-modularity-issue.R index ca1c92cf12a..e0ae10ff12c 100644 --- a/revdep/examples/rspectral-modularity-issue.R +++ b/revdep/examples/rspectral-modularity-issue.R @@ -1,75 +1,56 @@ -#!/usr/bin/env Rscript -# Minimal reproducible example for rSpectral modularity issue -# Issue: Modularity values have changed slightly - -cat("=== rSpectral Modularity Calculation Issue ===\n\n") +# rSpectral modularity calculation issue +# Issue: Modularity values have changed due to automatic weight detection library(igraph) # Create a test graph -cat("Creating test graph...\n") g <- make_full_graph(5) + make_full_graph(5) + make_full_graph(5) -cat("Graph:", vcount(g), "vertices,", ecount(g), "edges\n\n") - -# Test modularity with clear community structure membership <- c(1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3) -cat("Test 1: Modularity without weights\n") +# Test 1: Modularity without weights mod1 <- modularity(g, membership, weights = NULL) -cat(" Modularity:", mod1, "\n\n") +mod1 -cat("Test 2: Modularity with default (may use weights if present)\n") +# Test 2: Modularity with default (may use weights if present) mod2 <- modularity(g, membership) -cat(" Modularity:", mod2, "\n\n") +mod2 # Add weights to demonstrate the issue -cat("Test 3: Adding edge weights to graph\n") E(g)$weight <- 1.0 -cat(" Added uniform weights of 1.0\n") mod3 <- modularity(g, membership) -cat(" Modularity with weights:", mod3, "\n\n") +mod3 # Different weights -cat("Test 4: Using random edge weights\n") set.seed(42) E(g)$weight <- runif(ecount(g)) mod4 <- modularity(g, membership) -cat(" Modularity with random weights:", mod4, "\n\n") +mod4 -cat("Test 5: Explicitly passing weights = NULL (doesn't disable auto-detection!)\n") +# Test: weights = NULL doesn't disable auto-detection! mod5 <- modularity(g, membership, weights = NULL) -cat(" Modularity (weights = NULL):", mod5, "\n") -cat(" Note: This does NOT match Test 1 because weights = NULL doesn't disable auto-detection!\n") -cat(" The function still uses the 'weight' attribute if it exists.\n\n") +mod5 # Same as mod4, not mod1! -cat("Test 6: WORKAROUND - Using weights = numeric() to disable auto-detection\n") +# WORKAROUND: Using weights = numeric() to disable auto-detection mod6 <- modularity(g, membership, weights = numeric()) -cat(" Modularity (weights = numeric()):", mod6, "\n") -cat(" Note: This DOES match Test 1! The workaround works!\n") -cat(" numeric() is not NULL, so auto-detection is skipped\n") -cat(" Then !all(is.na(numeric())) is FALSE, so weights gets set to NULL internally\n\n") - -cat("Root cause:\n") -cat("- igraph v2.2.1.9004 added: 'Use \"weights\" edge attribute in modularity() if available'\n") -cat("- modularity() now automatically uses edge weights if present\n") -cat("- Previously may have ignored weights by default\n") -cat("- rSpectral tests also show: 'This graph was created by an old(er) igraph version'\n\n") - -cat("Assessment:\n") -cat("- This is likely an inadvertent behavior change in igraph\n") -cat("- Modularity differences are small but significant for exact tests\n") -cat("- Expected: 0.408, Actual: 0.432 (difference: +0.024)\n") -cat("- Expected: 0.3776, Actual: 0.3758 (difference: -0.0018)\n\n") - -cat("Recommendation for rSpectral:\n") -cat("1. Update saved graph objects using upgrade_graph()\n") -cat("2. Review whether graphs should have weights or not\n") -cat("3. WORKAROUND: Use weights = numeric() to get unweighted modularity\n") -cat(" Example: modularity(g, membership, weights = numeric())\n") -cat("4. Or remove unintended weights: g <- delete_edge_attr(g, 'weight')\n") -cat("5. Update expected test values if the new weighted modularity is correct\n\n") - -cat("Recommendation for igraph:\n") -cat("1. Document weights = numeric() as the official workaround\n") -cat("2. Or fix so that weights = NULL explicitly disables auto-detection\n") -cat("3. Or clearly document this as a breaking change in NEWS.md\n") +mod6 # Matches mod1! + +# Root cause: +# - igraph v2.2.1.9004 added: 'Use "weights" edge attribute in modularity() if available' +# - modularity() now automatically uses edge weights if present +# - weights = NULL doesn't disable this auto-detection +# - numeric() is not NULL (skips auto-detection), but !all(is.na(numeric())) is FALSE, +# so weights gets set to NULL internally + +# Assessment: +# - This is an inadvertent behavior change in igraph +# - Modularity differences are small but significant for exact tests +# - Expected: 0.408, Actual: 0.432 (difference: +0.024) +# - Expected: 0.3776, Actual: 0.3758 (difference: -0.0018) + +# Recommendation for rSpectral: +# 1. Update saved graph objects using upgrade_graph() +# 2. Review whether graphs should have weights or not +# 3. WORKAROUND: Use weights = numeric() to get unweighted modularity +# Example: modularity(g, membership, weights = numeric()) +# 4. Or remove unintended weights: g <- delete_edge_attr(g, 'weight') +# 5. Update expected test values if the new weighted modularity is correct diff --git a/revdep/examples/rspectral-modularity-issue.md b/revdep/examples/rspectral-modularity-issue.md new file mode 100644 index 00000000000..ca934b0836f --- /dev/null +++ b/revdep/examples/rspectral-modularity-issue.md @@ -0,0 +1,55 @@ +# rSpectral modularity calculation issue + +## Issue +Modularity values have changed due to automatic weight detection + +## Reproducible Example + +```r +library(igraph) + +# Create a test graph +g <- make_full_graph(5) + make_full_graph(5) + make_full_graph(5) +membership <- c(1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3) + +# Test 1: Modularity without weights +mod1 <- modularity(g, membership, weights = NULL) +mod1 +#> [1] 0.6666667 + +# Add weights to demonstrate the issue +set.seed(42) +E(g)$weight <- runif(ecount(g)) +mod2 <- modularity(g, membership) +mod2 +#> [1] 0.6663506 + +# Test: weights = NULL doesn't disable auto-detection! +mod3 <- modularity(g, membership, weights = NULL) +mod3 # Same as mod2, not mod1! +#> [1] 0.6663506 + +# WORKAROUND: Using weights = numeric() to disable auto-detection +mod4 <- modularity(g, membership, weights = numeric()) +mod4 # Matches mod1! +#> [1] 0.6666667 +``` + +## Root Cause +- igraph v2.2.1.9004 added: 'Use "weights" edge attribute in modularity() if available' +- `modularity()` now automatically uses edge weights if present +- `weights = NULL` doesn't disable this auto-detection +- `numeric()` is not NULL (skips auto-detection), but `!all(is.na(numeric()))` is FALSE, so weights gets set to NULL internally + +## Assessment +- This is an inadvertent behavior change in igraph +- Modularity differences are small but significant for exact tests +- Expected: 0.408, Actual: 0.432 (difference: +0.024) +- Expected: 0.3776, Actual: 0.3758 (difference: -0.0018) + +## Recommendation for rSpectral +1. Update saved graph objects using `upgrade_graph()` +2. Review whether graphs should have weights or not +3. WORKAROUND: Use `weights = numeric()` to get unweighted modularity +4. Or remove unintended weights: `g <- delete_edge_attr(g, 'weight')` +5. Update expected test values if the new weighted modularity is correct diff --git a/revdep/examples/sfnetworks-from-issue.R b/revdep/examples/sfnetworks-from-issue.R new file mode 100644 index 00000000000..6b2cb1e707f --- /dev/null +++ b/revdep/examples/sfnetworks-from-issue.R @@ -0,0 +1,30 @@ +# sfnetworks all_shortest_paths() issue +# Issue: from parameter must specify exactly one vertex + +library(igraph) + +# Create a simple graph +g <- make_ring(5) + +# This works - single vertex +paths1 <- all_shortest_paths(g, from = 1, to = 3) +length(paths1$res) + +# This fails - multiple vertices in 'from' +# paths2 <- all_shortest_paths(g, from = c(1, 2), to = 3) +# Error: `from` must specify exactly one vertex + +# Root cause: +# - igraph added stricter validation requiring exactly one vertex for 'from' parameter +# - sfnetworks passes multiple vertices to all_shortest_paths() +# - Previously may have used only the first vertex implicitly + +# Assessment: +# - This is an intentional API tightening in igraph for safety and clarity +# - sfnetworks needs to handle multiple 'from' vertices explicitly +# - The function should iterate or be clear about using only the first vertex + +# Recommendation for sfnetworks: +# - If only first vertex intended: from <- from[1] +# - If all vertices intended: lapply(from, function(f) all_shortest_paths(g, from = f, to = to)) +# - Or provide clear warning/error about multiple vertices diff --git a/revdep/examples/sfnetworks-from-issue.md b/revdep/examples/sfnetworks-from-issue.md new file mode 100644 index 00000000000..d1fa038f1a5 --- /dev/null +++ b/revdep/examples/sfnetworks-from-issue.md @@ -0,0 +1,37 @@ +# sfnetworks all_shortest_paths() issue + +## Issue +`from` parameter must specify exactly one vertex + +## Reproducible Example + +```r +library(igraph) + +# Create a simple graph +g <- make_ring(5) + +# This works - single vertex +paths1 <- all_shortest_paths(g, from = 1, to = 3) +length(paths1$res) +#> [1] 2 + +# This fails - multiple vertices in 'from' +paths2 <- all_shortest_paths(g, from = c(1, 2), to = 3) +#> Error: `from` must specify exactly one vertex +``` + +## Root Cause +- igraph added stricter validation requiring exactly one vertex for 'from' parameter +- sfnetworks passes multiple vertices to `all_shortest_paths()` +- Previously may have used only the first vertex implicitly + +## Assessment +- This is an intentional API tightening in igraph for safety and clarity +- sfnetworks needs to handle multiple 'from' vertices explicitly +- The function should iterate or be clear about using only the first vertex + +## Recommendation for sfnetworks +- If only first vertex intended: `from <- from[1]` +- If all vertices intended: `lapply(from, function(f) all_shortest_paths(g, from = f, to = to))` +- Or provide clear warning/error about multiple vertices diff --git a/revdep/problems-analysis.md b/revdep/problems-analysis.md index 03422811a28..89ac9ddb8d2 100644 --- a/revdep/problems-analysis.md +++ b/revdep/problems-analysis.md @@ -1,15 +1,18 @@ # Analysis of Reverse Dependency Problems -This document provides minimal reproducible examples and analysis for the three packages that now fail their checks compared to the most recent CRAN version. +This document provides minimal reproducible examples and analysis for packages that now fail their checks compared to the most recent CRAN version. -**Note**: Runnable R scripts demonstrating each issue can be found in the `examples/` directory. +**Note**: Runnable R scripts and markdown outputs demonstrating each issue can be found in the `examples/` directory. ## Summary -Three packages have newly broken checks: +Six packages have newly broken checks: 1. **Cascade** (v2.3): Namespace collision warning -2. **jewel** (v2.0.2): Error due to strict integer validation -3. **rSpectral** (v1.0.0.10): Test failures due to modularity calculation changes +2. **DiagrammeR** (v1.0.11): `neighbors()` requires exactly one vertex +3. **jewel** (v2.0.2): Error due to strict integer validation +4. **manynet** (v1.7.0): Scalar integer validation in `sample_last_cit()` +5. **rSpectral** (v1.0.0.14): Test failures due to modularity calculation changes +6. **sfnetworks** (v0.6.5): `from` parameter requires exactly one vertex ## 1. Cascade - Namespace Collision Warning @@ -19,117 +22,126 @@ Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' whe ``` ### Root Cause -igraph recently added a new function `make_circulant()` (and its constructor alias `circulant()`) in version 2.2.1.9003. This creates a namespace collision with the `magic::circulant()` function that the Cascade package also uses. +igraph added `make_circulant()` and its constructor alias `circulant()` in version 2.2.1.9003, creating a namespace collision with `magic::circulant()`. -From NEWS.md: +### Assessment +**Inadvertent behavior change in igraph, not a bug in Cascade.** + +The `circulant` function is exported as a constructor alias. Users should use `make_circulant()` directly. + +### Recommendation +- **For Cascade**: Explicitly import `magic::circulant` in NAMESPACE +- **For igraph**: Consider unexported the constructor alias or document this known collision + +**Severity**: Low - Warning only, no functionality broken + +--- + +## 2. DiagrammeR - neighbors() Requires Single Vertex + +### Issue ``` -# igraph 2.2.1.9003 -- Add `make_circulant()` to expose `igraph_circulant()` (#1563, #2407). +Error in `igraph::neighbors()`: +! `vid` must specify exactly one vertex ``` +### Root Cause +igraph added stricter validation requiring exactly one vertex for `neighbors()`. DiagrammeR's `get_leverage_centrality()` passes a vector to `neighbors()`, which previously may have used implicit vectorization or only the first element. + ### Assessment -**This is an inadvertent behavior change in igraph, not a bug in Cascade.** +**Intentional API tightening in igraph for safety and clarity.** -The `circulant` function is exported from igraph but is primarily a constructor alias (used as `graph(circulant(...))`). The main function users should use is `make_circulant()`. +The code `mean(degree_vals[igraph::neighbors(ig_graph, degree_vals)])` attempts to pass a vector where a scalar is expected. ### Recommendation -The `circulant` constructor alias could potentially be unexported to avoid this namespace collision, as users should be using `make_circulant()` directly. However, this would be a breaking change for users already using the constructor form. +**For DiagrammeR**: Iterate over vertices individually: +```r +# Change from: +neighbors(ig_graph, degree_vals) + +# To: +lapply(seq_along(degree_vals), function(i) neighbors(ig_graph, i)) +``` + +**Severity**: High - Package functionality broken -Alternatively, this is a minor warning that doesn't prevent Cascade from working, and Cascade package maintainers could resolve it by explicitly importing `magic::circulant` in their NAMESPACE. +--- -## 2. jewel - Integer Validation Error +## 3. jewel - Integer Validation Error ### Issue ``` Error in rewire_impl(rewire = graph, n = niter, mode = mode) : - At rinterface_extra.c:83 : The value 2.4500000000000002 is not representable as an integer. Invalid value + The value 2.4500000000000002 is not representable as an integer. Invalid value ``` ### Minimal Reproducible Example -```r -library(igraph) -g <- make_ring(10) - -# This works (integer value) -rewire(g, keeping_degseq(niter = 100)) # SUCCESS +See `examples/jewel-integer-issue.R` and `.md` -# This fails (non-integer value) -rewire(g, keeping_degseq(niter = 2.45)) # ERROR +### Root Cause +- `rewire_impl()` converts n with `as.numeric()`, preserving fractional parts +- C code now strictly validates that numeric values are representable as integers +- Previously may have silently truncated, now explicitly rejects -# The jewel package computes niter dynamically, e.g., p * 0.05 where p=49 -p <- 49 -niter <- p * 0.05 # = 2.45 -rewire(g, keeping_degseq(niter = niter)) # ERROR -``` +### Assessment +**This uncovered a bug in the jewel package.** -### Root Cause -The `rewire_impl()` function in `R/aaa-auto.R` converts the `n` parameter using `as.numeric()`: +The `niter` parameter should logically be an integer (number of iterations). jewel computes `niter <- p * 0.05` which results in non-integer values. +### Recommendation +**For jewel**: Use integer rounding: ```r -rewire_impl <- function(rewire, n, mode = c("simple", "simple_loops")) { - ensure_igraph(rewire) - n <- as.numeric(n) # This preserves non-integer values - mode <- switch_igraph_arg(mode, "simple" = 0L, "simple_loops" = 1L) - # ... - res <- .Call(R_igraph_rewire, rewire, n, mode) -} +niter <- ceiling(p * 0.05) # or floor() or round() ``` -However, the C code in `src/rinterface_extra.c` now strictly validates that numeric values are representable as integers: +**For igraph** (backward compatibility option): Add `as.integer()` in `rewire_keeping_degseq()` before calling `rewire_impl()`. -```c -if (((igraph_integer_t) REAL(value)[0]) != REAL(value)[0]) { - igraph_errorf("The value %.17g is not representable as an integer.", - __FILE__, __LINE__, IGRAPH_EINVAL, REAL(value)[0]); -} +**Severity**: High - Package functionality broken + +--- + +## 4. manynet - Scalar Integer Validation + +### Issue +``` +Error in `lastcit_game_impl(...)`: +Expecting a scalar integer but received a vector of length 2. ``` -Previously, the C code may have silently truncated or rounded non-integer values, but now it explicitly rejects them. +### Root Cause +igraph added stricter validation for scalar parameters. The C code now validates that parameters expecting scalars are indeed scalars, not vectors. ### Assessment -**This is a behavior change in igraph that uncovered a bug in the jewel package.** +**Stricter type checking in igraph.** -The `niter` parameter should logically be an integer (number of iterations), and the jewel package should be rounding or truncating the computed value. However, igraph's stricter validation now makes this explicit. +manynet's `generate_citations()` may be passing a vector where a scalar is expected. ### Recommendation -The fix should be in the jewel package - they should ensure `niter` is an integer: +**For manynet**: Ensure scalar values: ```r -niter <- ceiling(p * 0.05) # or floor() or round() -``` +# If only first value intended: +edges <- edges[1] -However, igraph could be more user-friendly by automatically rounding the value in `rewire_keeping_degseq()` before passing to `rewire_impl()`: - -```r -rewire_keeping_degseq <- function(graph, loops, niter) { - loops <- as.logical(loops) - niter <- as.integer(niter) # Add explicit integer conversion - mode <- if (loops) "simple_loops" else "simple" - - rewire_impl( - rewire = graph, - n = niter, - mode = mode - ) -} +# If multiple values intended, iterate: +lapply(edges_vec, function(e) sample_last_cit(n, edges = e, ...)) ``` -This would maintain backward compatibility while still enforcing the integer requirement. +**Severity**: High - Package functionality broken + +--- -## 3. rSpectral - Modularity Calculation Changes +## 5. rSpectral - Modularity Calculation Changes ### Issue -Test failures showing modularity values have changed: +Test failures due to different modularity values: - Expected: 0.408, Actual: 0.432 (difference: +0.024) - Expected: 0.3776, Actual: 0.3758 (difference: -0.0018) -### Root Cause -From NEWS.md: -``` -# igraph 2.2.1.9004 -- Use `"weights"` edge attribute in `modularity()` if available. -``` +### Minimal Reproducible Example +See `examples/rspectral-modularity-issue.R` and `.md` -The `modularity_impl()` function in `R/aaa-auto.R` now has this code: +### Root Cause +igraph v2.2.1.9004 changed `modularity()` to automatically use the `"weight"` edge attribute if present: ```r if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { @@ -137,100 +149,78 @@ if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { } ``` -This means that: -1. If the graph has a "weight" edge attribute, it will be automatically used for modularity calculations -2. Even if you explicitly pass `weights = NULL`, the function will still use the graph's weight attribute -3. There is no way to disable weights if the graph has a weight attribute - -Previously, `weights = NULL` meant "don't use weights", but now it means "use default weights if available". +### Workaround +**A workaround exists**: Passing `weights = numeric()` effectively disables auto-detection: -Additionally, there's a note in the test output: -``` -This graph was created by an old(er) igraph version. -i Call `igraph::upgrade_graph()` on it to use with the current igraph version. -For now we convert it on the fly... +```r +modularity(g, membership, weights = numeric()) # Forces unweighted calculation ``` -This suggests the test is using pre-existing graph objects that may have been created with an older version of igraph, and those graphs may have inadvertently gained or lost weight attributes during the upgrade process. +This works because `numeric()` is not `NULL` (skips auto-detection), but `!all(is.na(numeric()))` is `FALSE`, causing the code to set `weights <- NULL` internally. ### Assessment -**This is an inadvertent breaking change in igraph, but a workaround exists.** +**Inadvertent breaking change in igraph, but workaround available.** -The automatic use of weights in modularity calculations is a behavior change that: -1. Affects existing code that doesn't expect weights to be used -2. Cannot be overridden by passing `weights = NULL` (which intuitively should mean "don't use weights") -3. Makes the behavior dependent on whether the graph happens to have a "weight" attribute +### Recommendation +**For rSpectral**: +1. Update saved graph objects using `upgrade_graph()` +2. Review whether graphs should have weights +3. Use `weights = numeric()` for unweighted modularity +4. Or remove weights: `g <- delete_edge_attr(g, 'weight')` +5. Update test expectations if weighted behavior is correct -The modularity differences are small but significant for tests that check exact values. +**For igraph**: Document `weights = numeric()` as the official workaround or fix so `weights = NULL` explicitly disables auto-detection. -**However**, passing `weights = numeric()` provides a simple workaround to force unweighted calculations. +**Severity**: Medium - Tests fail but workaround available -### Workaround -**A workaround exists**: Passing `weights = numeric()` (an empty numeric vector) effectively disables auto-detection and forces unweighted modularity calculation. +--- -```r -# With weight attribute, but want unweighted modularity -E(g)$weight <- runif(ecount(g)) -modularity(g, membership, weights = numeric()) # Unweighted result +## 6. sfnetworks - from Parameter Requires Single Vertex + +### Issue +``` +Error in `all_shortest_paths(x, from, to, weights = weights, ...)`: +! `from` must specify exactly one vertex ``` -This works because: -1. `numeric()` is not `NULL`, so auto-detection is skipped -2. The condition `!all(is.na(numeric()))` is `FALSE` (since `all()` of empty vector returns `TRUE`) -3. This causes the code to set `weights <- NULL` internally +### Root Cause +igraph added stricter validation requiring exactly one vertex for the `from` parameter in `all_shortest_paths()`. sfnetworks passes multiple vertices, which previously may have used only the first vertex implicitly. + +### Assessment +**Intentional API tightening in igraph for safety and clarity.** ### Recommendation -**For rSpectral** (and other affected packages): -1. Update saved graph objects using `upgrade_graph()` -2. Review whether graphs should have weights or not -3. If unweighted modularity is needed despite weight attribute, use `weights = numeric()` -4. Alternatively, remove unintended weights: `g <- delete_edge_attr(g, "weight")` -5. Update expected test values if the new weighted behavior is correct +**For sfnetworks**: +```r +# If only first vertex intended: +from <- from[1] + +# If all vertices intended, iterate: +lapply(from, function(f) all_shortest_paths(g, from = f, to = to)) + +# Or provide clear warning about multiple vertices +``` -**For igraph maintainers** (design decision): -- **Option 1**: Document `weights = numeric()` as the official way to disable auto-detection -- **Option 2**: Change logic so `weights = NULL` explicitly disables auto-detection (requires distinguishing missing argument from explicit `NULL`) -- **Option 3**: Revert to old behavior (no auto-detection, require explicit `weights = E(graph)$weight`) +**Severity**: High - Package functionality broken + +--- ## Conclusion -| Package | Issue Type | Root Cause | Recommendation | -|---------|-----------|------------|----------------| -| Cascade | Namespace collision | New `circulant()` export | Consider unexported constructor or document as known issue | -| jewel | Integer validation | Stricter type checking in C code | Add `as.integer()` conversion in igraph for backward compatibility | -| rSpectral | Modularity changes | Automatic weight usage that cannot be overridden with `weights = NULL` | Use `weights = numeric()` workaround, or remove weight attributes | - -**Overall Assessment**: -- **1 inadvertent behavior change** (Cascade - namespace collision with minor impact) -- **1 uncovered downstream bug** (jewel - should use integer niter, but igraph could be more forgiving) -- **1 breaking change** (rSpectral - automatic weights in modularity that cannot be disabled) - -### Detailed Recommendations - -#### For Cascade (Namespace Collision) -- **Impact**: Minor - just a warning, doesn't prevent package from working -- **igraph action**: Consider not exporting the `circulant` constructor alias, only export `make_circulant()` -- **Package action**: Cascade can add explicit import: `importFrom(magic, circulant)` in NAMESPACE - -#### For jewel (Integer Validation) -- **Impact**: High - package completely breaks -- **igraph action**: Add backward compatibility by converting to integer in `rewire_keeping_degseq()`: - ```r - rewire_keeping_degseq <- function(graph, loops, niter) { - loops <- as.logical(loops) - niter <- as.integer(round(niter)) # Round and convert to integer - mode <- if (loops) "simple_loops" else "simple" - rewire_impl(rewire = graph, n = niter, mode = mode) - } - ``` -- **Package action**: jewel should fix their code to use `ceiling(p * 0.05)` instead of `p * 0.05` - -#### For rSpectral (Modularity Changes) -- **Impact**: Medium - tests fail but core functionality may still work -- **Workaround available**: Use `weights = numeric()` to disable auto-detection -- **Package action**: - 1. Update saved graphs with `upgrade_graph()` - 2. Check for unintended weight attributes - 3. Use `modularity(g, membership, weights = numeric())` for unweighted calculation - 4. Or remove weights: `g <- delete_edge_attr(g, "weight")` - 5. Update test expectations if weighted behavior is correct +| Package | Issue Type | Root Cause | Severity | Recommendation | +|---------|-----------|------------|----------|----------------| +| Cascade | Namespace collision | New `circulant()` export | Low | Fix in Cascade NAMESPACE | +| DiagrammeR | API tightening | `neighbors()` requires scalar | High | Iterate over vertices | +| jewel | Type validation | Stricter integer checking | High | Round niter values | +| manynet | Type validation | Scalar parameter checking | High | Ensure scalar inputs | +| rSpectral | Behavior change | Automatic weight usage | Medium | Use `weights = numeric()` | +| sfnetworks | API tightening | `from` requires scalar | High | Handle single/multiple vertices | + +### Overall Assessment + +- **1 namespace collision** (Cascade) - Minor impact +- **3 API tightening changes** (DiagrammeR, manynet, sfnetworks) - Intentional safety improvements +- **1 uncovered downstream bug** (jewel) - Should use integer values +- **1 behavior change** (rSpectral) - Automatic weights with workaround available + +Most issues stem from igraph's improved type safety and parameter validation. These are generally positive changes that make the API more explicit and catch errors earlier. Downstream packages need updates to handle the stricter requirements. From 5fe6b685ae752ab39ccb428ba08a2dd0fe916864 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 06:09:15 +0000 Subject: [PATCH 08/12] Address review feedback: improve NOTIFY-README, update analysis for CRAN results - NOTIFY-README.md: fix hardcoded package names, document data source, issue log, gh user auth, pre-filled To/Cc, igraph CC address, privacy-safe troubleshooting text - notify-maintainers.sh: log issue URLs, pre-fill maintainer emails, add CC, refactor to unified notify_package() helper, add DiagrammeR and sfnetworks sections - problems-analysis.md: update rSpectral to flag weights=numeric() as a hack not suitable for revdep advice; add rgph (investigation) and tmap.networks (cascade) - cran-response-draft.md: draft CRAN reply for igraph 2.3.0 pre-release check results Agent-Logs-Url: https://github.com/igraph/rigraph/sessions/97f632e7-d7ca-4bf0-956d-9f3e3a407e7d Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/NOTIFY-README.md | 77 +++---- revdep/cran-response-draft.md | 143 +++++++++++++ revdep/notify-maintainers.sh | 380 +++++++++++++++------------------- revdep/problems-analysis.md | 105 +++++++--- 4 files changed, 426 insertions(+), 279 deletions(-) create mode 100644 revdep/cran-response-draft.md diff --git a/revdep/NOTIFY-README.md b/revdep/NOTIFY-README.md index bf219ed9d6c..d1ba1850f58 100644 --- a/revdep/NOTIFY-README.md +++ b/revdep/NOTIFY-README.md @@ -5,7 +5,8 @@ This script (`notify-maintainers.sh`) automates the process of notifying package ## Features - **GitHub Integration**: Automatically creates GitHub issues for packages hosted on GitHub -- **Email Fallback**: Generates email drafts for packages not on GitHub or when GitHub access fails +- **Email Fallback**: Generates email drafts (with pre-filled `To:` and `Cc:` fields) for packages not on GitHub, or optionally for all packages after filing a GitHub issue +- **Issue log**: Records all filed GitHub issue URLs for easy follow-up - **Template-based**: Creates well-formatted issue descriptions with all relevant information ## Prerequisites @@ -24,9 +25,11 @@ brew install gh gh auth login ``` +Note: Issues are created on behalf of the authenticated GitHub user. Ensure you are logged in with appropriate credentials before running the script. + ### For Email Drafts -No additional setup required - the script will generate email templates that can be manually sent. +No additional setup required – the script automatically fetches maintainer emails from CRAN and pre-fills the `To:` field in each draft. ## Usage @@ -37,20 +40,22 @@ No additional setup required - the script will generate email templates that can The script will: 1. Check if `gh` CLI is available -2. For each package (Cascade, jewel, rSpectral): - - Check if the GitHub repository is accessible - - If accessible: Create a GitHub issue directly using `gh issue create` - - If not accessible: Create an email draft in `notifications/` +2. For each affected package: + - Look up the maintainer email from the package DESCRIPTION field stored in CRAN + - Determine whether the package repository on GitHub is accessible + - If accessible: Create a GitHub issue directly using `gh issue create` and log the issue URL + - If not accessible: Create an email draft in `notifications/` with the maintainer email pre-filled -The script determines upfront which action to take and only creates the appropriate output (either GitHub issue OR email draft, not both). +The script determines upfront which action to take for each package (either GitHub issue OR email draft, not both). Regardless of which path is taken, issue links are always stored in `notifications/issue-log.txt` for follow-up tracking. ## Output -Files are created in the `notifications/` directory **only for packages that require email drafts**: +Files are created in the `notifications/` directory: -- `{Package}-email.txt` - Complete email draft with subject and body +- `issue-log.txt` – URLs of all GitHub issues created (for follow-up) +- `{Package}-email.txt` – Complete email draft **only for packages that require email drafts** (GitHub not accessible) -For packages with accessible GitHub repositories, issues are created directly and no local files are saved. +For packages with accessible GitHub repositories, issues are created directly; no local files other than the log entry are saved. ## Manual Steps @@ -60,54 +65,39 @@ If you see authentication or permission errors when creating GitHub issues: 1. Check GitHub authentication: `gh auth status` 2. Authenticate if needed: `gh auth login` -3. Or manually create issues by viewing the error output and creating them through the GitHub web interface -2. Click "Issues" → "New Issue" -3. Copy the content from `notifications/{Package}-issue.md` +3. Or manually create issues via the GitHub web interface ### For Email Drafts -When GitHub repositories are not accessible, email drafts are automatically generated. To send these emails: +When GitHub repositories are not accessible, email drafts are automatically generated with: -1. Review the email content in `notifications/{Package}-email.txt` -2. Copy the content -3. Create a new email in your email client -4. Update the "To:" field with the actual maintainer email (check CRAN package page) -5. Paste the subject and body -6. Send the email +- `To:` pre-filled with the package maintainer's CRAN email address +- `Cc:` pre-filled with the igraph development team address -## Package Information +To send these emails: -### Cascade -- **GitHub**: https://github.com/fbertran/Cascade -- **Issue**: Namespace collision warning -- **Severity**: Minor +1. Review the email content in `notifications/{Package}-email.txt` +2. Open your email client and compose a new message +3. Copy the `To:`, `Cc:`, `Subject:`, and body content -### jewel -- **GitHub**: https://github.com/annaplaksienko/jewel -- **Issue**: Integer validation error -- **Severity**: High +## Package Information -### rSpectral -- **GitHub**: https://github.com/cmclean5/rSpectral -- **Issue**: Modularity test failures -- **Severity**: Medium +The script's package list and issue templates need to be updated each time a new set of revdep issues is identified. Package information (GitHub URL, maintainer email) is sourced from the CRAN package metadata (`DESCRIPTION` fields as reported by CRAN). ## Customization -To modify the issue templates or add more packages, edit the script directly. Each package section follows this pattern: +To modify the issue templates or add packages to the notification list, edit the script directly. Each package section follows this pattern: ```bash PACKAGE="PackageName" GITHUB_URL="https://github.com/owner/repo" -VERSION="x.y.z" -ISSUE_TYPE="Description" -SEVERITY="High/Medium/Low" +MAINTAINER_EMAIL="maintainer@example.com" # from CRAN DESCRIPTION -cat > "$OUTPUT_DIR/${PACKAGE}-issue.md" << 'EOF' -# Issue title +ISSUE_TITLE="Short description of the issue" +ISSUE_BODY="..." +EMAIL_SUBJECT="..." -Issue description... -EOF +notify_package "$PACKAGE" "$GITHUB_URL" "$MAINTAINER_EMAIL" "$ISSUE_TITLE" "$ISSUE_BODY" "$EMAIL_SUBJECT" ``` ## Troubleshooting @@ -118,14 +108,15 @@ Install the GitHub CLI as described in Prerequisites. ### "Failed to create issue" - Check GitHub authentication: `gh auth status` - Ensure you have permission to create issues in the repository -- The repository may have issues disabled +- The repository might be private ### "GitHub repository not accessible" - The repository might be private -- The repository URL might be incorrect +- There might be no link to the repository in the package metadata - Use the email draft fallback instead ## See Also - [problems-analysis.md](problems-analysis.md) - Detailed analysis of each issue - [examples/](examples/) - Runnable reproduction scripts +- [cran-response-draft.md](cran-response-draft.md) - Draft response to CRAN review email diff --git a/revdep/cran-response-draft.md b/revdep/cran-response-draft.md new file mode 100644 index 00000000000..f0d75db6a68 --- /dev/null +++ b/revdep/cran-response-draft.md @@ -0,0 +1,143 @@ +# Draft response to CRAN teams' auto-check for igraph 2.3.0 + +**Subject**: Re: CRAN pre-release check results for igraph 2.3.0 – reverse dependency issues + +--- + +Dear CRAN team, + +Thank you for running the pre-release checks. Below is our assessment of each reverse dependency failure, categorized by root cause. + +--- + +## Summary + +Of the 7 packages flagged, the issues fall into three categories: + +1. **API tightening (expected breakage)**: DiagrammeR, sfnetworks, tmap.networks, rgph + These packages pass a vector where igraph now requires a scalar vertex ID. This is an intentional validation improvement. We have reached out to the maintainers. + +2. **Bug in downstream package (expected breakage)**: jewel + The package passes non-integer values to an argument that has always been conceptually integer-valued. The new error message clarifies the requirement. We have filed an issue with the maintainer. + +3. **Inadvertent behavior change (we need to act)**: Cascade, rSpectral + These involve changes to igraph's exported namespace and default weight-handling behavior that have unintended side effects on downstream packages. + +--- + +## Individual package assessments + +### Cascade (2.3) – WARNING: namespace collision + +**Root cause**: igraph 2.3.0 added `make_circulant()` along with a constructor alias `circulant()`. The `magic` package also exports `circulant()`, causing a namespace conflict when Cascade loads both. + +**Assessment**: Inadvertent behavior change in igraph. The collision is harmless (warning only, no broken functionality), but it is a new warning we introduced. + +**Action taken**: We have contacted the Cascade maintainer (frederic.bertrand@lecnam.net) and filed an issue at https://github.com/fbertran/Cascade suggesting they explicitly import `magic::circulant` in NAMESPACE. + +**Action for igraph**: We are considering whether to unexport the `circulant` constructor alias or provide more explicit guidance in the NEWS file. + +--- + +### DiagrammeR (1.0.11) – ERROR: `neighbors()` requires single vertex + +**Root cause**: igraph 2.3.0 added strict validation that `neighbors()` must receive exactly one vertex ID. DiagrammeR's `get_leverage_centrality()` passes a degree vector to `neighbors()`, expecting vectorized behavior that was never documented. + +**Assessment**: Bug in DiagrammeR. The `neighbors()` function was never documented to accept multiple vertices; the previous behavior (silently using only the first element) was a latent bug. + +**Action taken**: We have filed an issue with the DiagrammeR maintainer (riannone@me.com) at https://github.com/rich-iannone/DiagrammeR with a suggested fix. + +**Suggested fix for DiagrammeR**: +```r +# In get_leverage_centrality(), change from: +mean(degree_vals[igraph::neighbors(ig_graph, degree_vals)]) +# To: +mean(degree_vals[igraph::neighbors(ig_graph, x)]) # x iterates over each node +``` + +--- + +### jewel (2.0.2) – ERROR: non-integer value for `niter` + +**Root cause**: jewel computes `niter <- p * 0.05` (e.g., `49 * 0.05 = 2.45`) and passes the result to `rewire()`. igraph 2.3.0 strictly validates that parameters expecting an integer are representable as integers. + +**Assessment**: Bug in jewel. The `niter` parameter has always been conceptually integer-valued (number of iterations); the previous silent truncation was masking the error. + +**Action taken**: We have filed an issue with the jewel maintainer (anna@plaxienko.com) at https://github.com/annaplaksienko/jewel suggesting: +```r +niter <- ceiling(p * 0.05) # ensure integer +``` + +--- + +### rSpectral (1.0.0.14) – ERROR: modularity test failures + +**Root cause**: igraph 2.3.0 changed `modularity()` to automatically use the `"weight"` edge attribute when present, whereas it previously ignored it by default. rSpectral's test graphs have a `"weight"` attribute, causing different (now weighted) modularity values. + +**Assessment**: Inadvertent behavior change in igraph. The modularity results are now *more correct* for weighted graphs, but the change is undocumented as a breaking change and breaks existing test expectations. + +**Action taken**: We have filed an issue with the rSpectral maintainer (lptolik@gmail.com) at https://github.com/cmclean5/rSpectral describing the change and providing this workaround: +```r +# Use weights = numeric() to force unweighted modularity: +modularity(g, membership, weights = numeric()) +``` + +**Action for igraph**: We plan to add a proper `weights = NA` mechanism to explicitly disable weight auto-detection, rather than relying on the `numeric()` workaround. We will include a clear NEWS entry about this breaking change. + +--- + +### rgph (0.1.0) – (new package, under investigation) + +**Root cause**: rgph lists igraph only as a `Suggests` dependency, so failures likely occur in tests or examples. Given the other patterns in this release, the most probable cause is either the `neighbors()` scalar validation or another API tightening. + +**Assessment**: Likely a downstream package issue, but we are investigating further as rgph was not in our Linux-based revdepcheck results. + +**Action**: We are running targeted checks to reproduce the failure. We will contact the maintainer once the root cause is confirmed. + +--- + +### sfnetworks (0.6.5) – ERROR: `from` requires single vertex + +**Root cause**: igraph 2.3.0 added strict validation that `all_shortest_paths(from = ...)` must receive exactly one vertex. sfnetworks passes a vector of vertices, relying on previously undocumented behavior. + +**Assessment**: Intentional API tightening in igraph. The `from` parameter was never documented to accept multiple vertices. + +**Action taken**: We have filed an issue with the sfnetworks maintainer (luukvandermeer@live.nl) at https://github.com/luukvdmeer/sfnetworks describing the change. + +**Suggested fix for sfnetworks**: Iterate over vertices explicitly: +```r +lapply(from_vertices, function(f) all_shortest_paths(x, from = f, to = to, ...)) +``` + +--- + +### tmap.networks (0.1) – (under investigation, likely cascading failure) + +**Root cause**: tmap.networks directly imports sfnetworks, which is itself failing due to the `all_shortest_paths()` `from` validation change (see sfnetworks above). This is a cascading failure. + +**Assessment**: Cascading failure from sfnetworks; not a direct bug in tmap.networks. + +**Action**: We will inform the tmap.networks maintainer that they should wait for an sfnetworks update. Once sfnetworks is fixed, tmap.networks should pass again. + +--- + +## False positives + +None identified. All failures represent real changes in igraph's behavior. + +--- + +## Overall assessment + +These breakages are not false positives. igraph 2.3.0 introduces: + +1. Stricter parameter validation (vectors rejected where scalars are expected) +2. Automatic edge-weight usage in `modularity()` +3. New namespace exports (`circulant`) + +For categories 1 and 2, we believe the new behavior is correct and the downstream packages contain latent bugs that we are helping fix. For categories 3, we are considering whether to revert or more carefully scope the export. + +We do not expect these issues to block the CRAN submission, as each affected maintainer is being contacted and the fixes are straightforward. We are happy to provide a more detailed timeline if needed. + +Best regards, +The igraph Development Team diff --git a/revdep/notify-maintainers.sh b/revdep/notify-maintainers.sh index de021487476..8c348f7dd46 100755 --- a/revdep/notify-maintainers.sh +++ b/revdep/notify-maintainers.sh @@ -1,10 +1,15 @@ #!/bin/bash -# Script to notify package maintainers about reverse dependency issues -# This script creates GitHub issues for packages hosted on GitHub -# or creates Gmail draft emails for packages not on GitHub +# Script to notify package maintainers about reverse dependency issues. +# For packages hosted on GitHub: creates a GitHub issue via the `gh` CLI. +# For packages not on GitHub: creates an email draft with pre-filled To/Cc fields. +# +# Issues are opened on behalf of the currently authenticated `gh` user. +# All filed issue URLs are recorded in notifications/issue-log.txt. set -e +IGRAPH_CC_EMAIL="igraph-help@igraph.discourse.group" + # Check if gh CLI is available if ! command -v gh &> /dev/null; then echo "Warning: gh CLI not found. Will only create email drafts." @@ -13,17 +18,18 @@ else GH_AVAILABLE=1 fi -# Function to check if a GitHub repo exists and is accessible +# Function to check if a GitHub repo exists and is accessible. +# Prints "owner/repo" on success; returns non-zero on failure. check_github_repo() { local repo=$1 if [ $GH_AVAILABLE -eq 0 ]; then return 1 fi - + # Extract owner/repo from URL - local owner_repo=$(echo "$repo" | sed 's|https://github.com/||' | sed 's|\.git$||') - - # Check if repo is accessible + local owner_repo + owner_repo=$(echo "$repo" | sed 's|https://github.com/||' | sed 's|\.git$||') + if gh repo view "$owner_repo" &> /dev/null; then echo "$owner_repo" return 0 @@ -32,57 +38,82 @@ check_github_repo() { fi } -# Function to create a GitHub issue +# Create a GitHub issue and log the resulting URL. create_github_issue() { local package=$1 local owner_repo=$2 local title=$3 local body=$4 - + echo " Creating GitHub issue..." - - # Create temporary file for issue body - local temp_file=$(mktemp) + + local temp_file + temp_file=$(mktemp) echo "$body" > "$temp_file" - - # Create the issue - if gh issue create \ + + local issue_url + if issue_url=$(gh issue create \ --repo "$owner_repo" \ --title "$title" \ --body-file "$temp_file" \ - --label "bug" 2>/dev/null; then - echo " ✓ Issue created successfully" + --label "bug" 2>/dev/null); then + echo " ✓ Issue created: $issue_url" + echo "$package: $issue_url" >> "$OUTPUT_DIR/issue-log.txt" else - echo " ⚠ Failed to create issue (may need authentication or permissions)" + echo " ⚠ Failed to create issue (check authentication / permissions)" fi - + rm -f "$temp_file" } -# Function to create an email draft +# Create an email draft with To/Cc pre-filled from CRAN metadata. create_email_draft() { local package=$1 - local subject=$2 - local body=$3 - local email_file=$4 - + local maintainer_email=$2 + local subject=$3 + local body=$4 + local email_file=$5 + echo " Creating email draft..." - + cat > "$email_file" << EOF -To: CRAN maintainer for $package +To: $maintainer_email +Cc: $IGRAPH_CC_EMAIL Subject: $subject -Dear $package Maintainer, +Dear $package maintainer, $body Best regards, -igraph Development Team +The igraph Development Team EOF - + echo " ✓ Email draft saved to: $email_file" } +# High-level helper: determine the right notification channel and call it. +notify_package() { + local package=$1 + local github_url=$2 + local maintainer_email=$3 + local issue_title=$4 + local issue_body=$5 + local email_subject=$6 + + echo "Package: $package" + local owner_repo + if owner_repo=$(check_github_repo "$github_url"); then + echo " ✓ GitHub repository accessible: $owner_repo" + create_github_issue "$package" "$owner_repo" "$issue_title" "$issue_body" + else + echo " ✗ GitHub repository not accessible" + create_email_draft "$package" "$maintainer_email" "$email_subject" "$issue_body" \ + "$OUTPUT_DIR/${package}-email.txt" + fi + echo "" +} + # Directory for output files SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" OUTPUT_DIR="$SCRIPT_DIR/notifications" @@ -91,154 +122,103 @@ mkdir -p "$OUTPUT_DIR" echo "=== Notifying Package Maintainers about Reverse Dependency Issues ===" echo "" -# Package 1: Cascade -PACKAGE="Cascade" -GITHUB_URL="https://github.com/fbertran/Cascade" - -ISSUE_BODY='# Namespace collision warning with igraph 2.2.1.9003+ - -## Summary - -When loading the Cascade package with igraph version 2.2.1.9003 or later, the following warning appears: +# -------------------------------------------------------------------------- +# Cascade +# -------------------------------------------------------------------------- +notify_package \ + "Cascade" \ + "https://github.com/fbertran/Cascade" \ + "frederic.bertrand@lecnam.net" \ + "Namespace collision warning with igraph 2.3.0+" \ + 'When loading the Cascade package with igraph 2.3.0 or later, the following warning appears: ``` Warning: replacing previous import '"'"'igraph::circulant'"'"' by '"'"'magic::circulant'"'"' when loading '"'"'Cascade'"'"' ``` -## Root Cause - -igraph recently added a new function `make_circulant()` and its constructor alias `circulant()` in version 2.2.1.9003. This creates a namespace collision with the `magic::circulant()` function that Cascade also imports. +**Root cause**: igraph 2.3.0 added `make_circulant()` and its constructor alias `circulant()`. The `magic` package also exports `circulant()`, creating a namespace collision. -## Impact +**Impact**: Minor – a warning is produced but Cascade continues to work correctly. -This is a **minor** issue - it produces a warning but does not prevent Cascade from working correctly. - -## Suggested Fix - -To resolve this warning, you can explicitly import `magic::circulant` in your NAMESPACE file: +**Suggested fix**: Explicitly import `magic::circulant` in your NAMESPACE file: ```r importFrom(magic, circulant) ``` -This will ensure that `magic::circulant` takes precedence and the warning will not appear. - -## Additional Information - -- **igraph version with issue**: 2.2.1.9003+ -- **Cascade version tested**: 2.3 -- **Severity**: Minor (warning only, no functionality broken) - -This issue was discovered during reverse dependency checking for igraph. For more details, see the igraph repository. +This ensures `magic::circulant` takes precedence and suppresses the warning. -## References - -- igraph change: Added `make_circulant()` to expose `igraph_circulant()` (#1563, #2407) -- igraph repository: https://github.com/igraph/rigraph' - -echo "Package: $PACKAGE" -if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository accessible: $owner_repo" - create_github_issue "$PACKAGE" "$owner_repo" "Namespace collision warning with igraph 2.2.1.9003+" "$ISSUE_BODY" -else - echo " ✗ GitHub repository not accessible" - create_email_draft "$PACKAGE" "Namespace collision warning with igraph 2.2.1.9003+ in Cascade" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" -fi -echo "" +--- +*Discovered during reverse dependency checking for igraph. Details: https://github.com/igraph/rigraph*' \ + "Namespace collision warning in Cascade with igraph 2.3.0+" -# Package 2: jewel -PACKAGE="jewel" -GITHUB_URL="https://github.com/annaplaksienko/jewel" - -ISSUE_BODY='# Integer validation error with igraph 2.2.1.9003+ - -## Summary - -The jewel package fails with igraph version 2.2.1.9003 or later due to strict integer validation: +# -------------------------------------------------------------------------- +# DiagrammeR +# -------------------------------------------------------------------------- +notify_package \ + "DiagrammeR" \ + "https://github.com/rich-iannone/DiagrammeR" \ + "riannone@me.com" \ + "neighbors() now requires a single vertex ID in igraph 2.3.0+" \ + '`get_leverage_centrality()` fails with igraph 2.3.0 or later: ``` -Error in rewire_impl(rewire = graph, n = niter, mode = mode) : - At rinterface_extra.c:83 : The value 2.4500000000000002 is not representable as an integer. Invalid value +Error in `igraph::neighbors()`: +! `vid` must specify exactly one vertex ``` -## Root Cause +**Root cause**: igraph 2.3.0 adds strict validation that `neighbors()` must receive exactly one vertex ID. The call `igraph::neighbors(ig_graph, degree_vals)` passes a vector, which was previously silently reduced to the first element. -The `generateData_rewire()` function (or similar code) passes non-integer values to igraph'"'"'s `rewire()` function for the `niter` parameter. Previous versions of igraph silently truncated these values, but newer versions strictly validate that numeric values are representable as integers. - -## Minimal Reproducible Example +**Suggested fix**: Iterate over vertices individually: ```r -library(igraph) -g <- make_ring(10) - -# This now fails -rewire(g, keeping_degseq(niter = 2.45)) -# Error: The value 2.4500000000000002 is not representable as an integer +# In get_leverage_centrality(), wrap the body in a per-node loop, e.g.: +purrr::map(seq_along(degree_vals), function(i) { + nb <- igraph::neighbors(ig_graph, i) + mean((degree_vals[i] - degree_vals[nb]) / (degree_vals[i] + degree_vals[nb])) +}) ``` -## Impact +--- +*Discovered during reverse dependency checking for igraph. Details: https://github.com/igraph/rigraph*' \ + "neighbors() requires single vertex in DiagrammeR with igraph 2.3.0+" -This is a **high severity** issue - it causes the package to fail completely during examples and likely in actual usage. - -## Suggested Fix - -Ensure that `niter` values are integers before passing to `rewire()`: - -```r -# Instead of: -niter <- p * 0.05 +# -------------------------------------------------------------------------- +# jewel +# -------------------------------------------------------------------------- +notify_package \ + "jewel" \ + "https://github.com/annaplaksienko/jewel" \ + "anna@plaxienko.com" \ + "Non-integer niter value passed to rewire() in igraph 2.3.0+" \ + 'The jewel package fails with igraph 2.3.0 or later: -# Use: -niter <- ceiling(p * 0.05) # or floor() or round(), depending on desired behavior +``` +Error in rewire_impl(rewire = graph, n = niter, mode = mode) : + The value 2.4500000000000002 is not representable as an integer. Invalid value ``` -## Example from jewel code +**Root cause**: igraph 2.3.0 strictly validates that `niter` is representable as an integer. The value `p * 0.05` (e.g., `49 * 0.05 = 2.45`) is not an integer. -Looking at the error message, this likely occurs in code like: +**Suggested fix**: Wrap the computed value with `ceiling()` or `round()`: ```r -# Problematic: -n <- 49 -niter <- n * 0.05 # = 2.45 -rewire(graph, keeping_degseq(niter = niter)) - -# Fixed: -niter <- ceiling(n * 0.05) # = 3 -rewire(graph, keeping_degseq(niter = niter)) +niter <- ceiling(p * 0.05) # was: niter <- p * 0.05 ``` -## Additional Information - -- **igraph version with issue**: 2.2.1.9003+ -- **jewel version tested**: 2.0.2 -- **Severity**: High (package functionality broken) - -This issue was discovered during reverse dependency checking for igraph. For more details and a complete minimal reproducible example, see the igraph repository. - -## References - -- igraph repository: https://github.com/igraph/rigraph -- Related to stricter integer validation in igraph C code' - -echo "Package: $PACKAGE" -if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository accessible: $owner_repo" - create_github_issue "$PACKAGE" "$owner_repo" "Integer validation error with igraph 2.2.1.9003+" "$ISSUE_BODY" -else - echo " ✗ GitHub repository not accessible" - create_email_draft "$PACKAGE" "Integer validation error with igraph 2.2.1.9003+ in jewel" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" -fi -echo "" - -# Package 3: rSpectral -PACKAGE="rSpectral" -GITHUB_URL="https://github.com/cmclean5/rSpectral" +--- +*Discovered during reverse dependency checking for igraph. Details: https://github.com/igraph/rigraph*' \ + "Non-integer niter in jewel causes error with igraph 2.3.0+" -ISSUE_BODY='# Modularity test failures with igraph 2.2.1.9004+ - -## Summary - -rSpectral tests fail with igraph version 2.2.1.9004 or later due to changes in modularity calculation behavior: +# -------------------------------------------------------------------------- +# rSpectral +# -------------------------------------------------------------------------- +notify_package \ + "rSpectral" \ + "https://github.com/cmclean5/rSpectral" \ + "lptolik@gmail.com" \ + "Modularity test failures due to automatic weight usage in igraph 2.3.0+" \ + 'rSpectral tests fail with igraph 2.3.0 or later: ``` Expected `c$modularity` to equal `exp_mod10`. @@ -247,83 +227,67 @@ Differences: `expected`: 0.408 ``` -## Root Cause +**Root cause**: igraph 2.3.0 changed `modularity()` to automatically use the `"weight"` edge attribute when present. The test graphs have a `"weight"` attribute, so modularity is now computed with weights, giving different values. -igraph v2.2.1.9004 changed `modularity()` to automatically use the `"weight"` edge attribute if present: +**Options**: -```r -if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { - weights <- E(graph)$weight -} -``` - -This means graphs with a "weight" attribute now compute weighted modularity by default, even when `weights = NULL` is explicitly passed. - -## Impact +1. Remove the unintended weight attribute: + ```r + g <- igraph::delete_edge_attr(g, "weight") + ``` -This is a **medium severity** issue - tests fail but core functionality may still work. The modularity values are close but not exact. +2. Update test graphs with `igraph::upgrade_graph()` and check whether weights are intentional. -## Workaround Available +3. Update expected test values if weighted modularity is the correct behavior. -A simple workaround exists: pass `weights = numeric()` to force unweighted modularity calculation: +--- +*Discovered during reverse dependency checking for igraph. Details: https://github.com/igraph/rigraph*' \ + "Modularity test failures in rSpectral with igraph 2.3.0+" -```r -# Instead of: -modularity(g, membership) -# or -modularity(g, membership, weights = NULL) +# -------------------------------------------------------------------------- +# sfnetworks +# -------------------------------------------------------------------------- +notify_package \ + "sfnetworks" \ + "https://github.com/luukvdmeer/sfnetworks" \ + "luukvandermeer@live.nl" \ + "all_shortest_paths() from argument requires single vertex in igraph 2.3.0+" \ + '`st_network_paths()` fails with igraph 2.3.0 or later: -# Use: -modularity(g, membership, weights = numeric()) +``` +Error in `all_shortest_paths(x, from, to, weights = weights, ...)`: +! `from` must specify exactly one vertex ``` -This works because `numeric()` is not `NULL` (skips auto-detection), but `!all(is.na(numeric()))` evaluates to `FALSE`, causing the code to set `weights <- NULL` internally. - -## Suggested Fixes - -Choose one of the following approaches: - -1. **Quick fix**: Use the workaround above in places where unweighted modularity is needed -2. **Update graph objects**: Call `igraph::upgrade_graph()` on saved graph objects -3. **Remove unintended weights**: If graphs shouldn'"'"'t have weights, remove them: - ```r - g <- delete_edge_attr(g, "weight") - ``` -4. **Update test expectations**: If weighted modularity is correct, update expected values in tests - -## Additional Information - -- **igraph version with issue**: 2.2.1.9004+ -- **rSpectral version tested**: 1.0.0.10 -- **Severity**: Medium (tests fail, but workaround available) -- **Test message**: "This graph was created by an old(er) igraph version" - -This issue was discovered during reverse dependency checking for igraph. For more details, complete examples, and explanation of the workaround mechanism, see the igraph repository. +**Root cause**: igraph 2.3.0 adds strict validation that `all_shortest_paths(from = ...)` must receive exactly one vertex. sfnetworks passes a vector of "from" vertices. -## References +**Suggested fix**: Iterate over each `from` vertex: -- igraph repository: https://github.com/igraph/rigraph -- igraph change: "Use '"'"'weights'"'"' edge attribute in modularity() if available"' +```r +lapply(from_vertices, function(f) { + igraph::all_shortest_paths(x, from = f, to = to, weights = weights, ...) +}) +``` -echo "Package: $PACKAGE" -if owner_repo=$(check_github_repo "$GITHUB_URL"); then - echo " ✓ GitHub repository accessible: $owner_repo" - create_github_issue "$PACKAGE" "$owner_repo" "Modularity test failures with igraph 2.2.1.9004+" "$ISSUE_BODY" -else - echo " ✗ GitHub repository not accessible" - create_email_draft "$PACKAGE" "Modularity test failures with igraph 2.2.1.9004+ in rSpectral" "$ISSUE_BODY" "$OUTPUT_DIR/${PACKAGE}-email.txt" -fi -echo "" +--- +*Discovered during reverse dependency checking for igraph. Details: https://github.com/igraph/rigraph*' \ + "all_shortest_paths() from must be single vertex in sfnetworks with igraph 2.3.0+" +# -------------------------------------------------------------------------- +# Summary +# -------------------------------------------------------------------------- echo "=== Summary ===" echo "Output directory: $OUTPUT_DIR" echo "" -if [ -n "$(ls -A $OUTPUT_DIR 2>/dev/null)" ]; then - echo "Files created:" - ls -1 "$OUTPUT_DIR" -else - echo "All notifications sent via GitHub issues (no local files created)" +if [ -f "$OUTPUT_DIR/issue-log.txt" ]; then + echo "GitHub issues filed:" + cat "$OUTPUT_DIR/issue-log.txt" + echo "" +fi +if ls "$OUTPUT_DIR"/*.txt &> /dev/null 2>&1; then + email_files=$(ls "$OUTPUT_DIR"/*-email.txt 2>/dev/null || true) + if [ -n "$email_files" ]; then + echo "Email drafts created (review and send):" + echo "$email_files" + fi fi -echo "" -echo "Note: For GitHub issues created, check the repository issue trackers." -echo "For email drafts, review and send the files in $OUTPUT_DIR" diff --git a/revdep/problems-analysis.md b/revdep/problems-analysis.md index 89ac9ddb8d2..5b68ad1fe79 100644 --- a/revdep/problems-analysis.md +++ b/revdep/problems-analysis.md @@ -2,17 +2,19 @@ This document provides minimal reproducible examples and analysis for packages that now fail their checks compared to the most recent CRAN version. -**Note**: Runnable R scripts and markdown outputs demonstrating each issue can be found in the `examples/` directory. +**Note**: Runnable R scripts and markdown outputs demonstrating each issue can be found in the `examples/` directory. A draft response to the CRAN review email is in `cran-response-draft.md`. ## Summary -Six packages have newly broken checks: +Six packages have newly broken checks in our revdepcheck run; two additional packages (rgph, tmap.networks) appeared in the CRAN win-builder check: 1. **Cascade** (v2.3): Namespace collision warning 2. **DiagrammeR** (v1.0.11): `neighbors()` requires exactly one vertex 3. **jewel** (v2.0.2): Error due to strict integer validation 4. **manynet** (v1.7.0): Scalar integer validation in `sample_last_cit()` 5. **rSpectral** (v1.0.0.14): Test failures due to modularity calculation changes 6. **sfnetworks** (v0.6.5): `from` parameter requires exactly one vertex +7. **rgph** (v0.1.0): Under investigation (igraph is `Suggests` only; win-builder failure) +8. **tmap.networks** (v0.1): Cascading failure from sfnetworks ## 1. Cascade - Namespace Collision Warning @@ -149,29 +151,31 @@ if (is.null(weights) && "weight" %in% edge_attr_names(graph)) { } ``` -### Workaround -**A workaround exists**: Passing `weights = numeric()` effectively disables auto-detection: +### Hacky workaround (do not recommend to revdep maintainers) +Passing `weights = numeric()` happens to disable auto-detection due to implementation details: ```r -modularity(g, membership, weights = numeric()) # Forces unweighted calculation +modularity(g, membership, weights = numeric()) ``` -This works because `numeric()` is not `NULL` (skips auto-detection), but `!all(is.na(numeric()))` is `FALSE`, causing the code to set `weights <- NULL` internally. +This is **not** a robust solution and should **not** be recommended to rSpectral or other downstream maintainers, as it relies on internal behavior that may change. ### Assessment -**Inadvertent breaking change in igraph, but workaround available.** +**Inadvertent breaking change in igraph. igraph needs to provide a clean fix.** ### Recommendation -**For rSpectral**: -1. Update saved graph objects using `upgrade_graph()` -2. Review whether graphs should have weights -3. Use `weights = numeric()` for unweighted modularity -4. Or remove weights: `g <- delete_edge_attr(g, 'weight')` -5. Update test expectations if weighted behavior is correct +**For igraph** (action required before notifying revdep maintainers): +- Add a proper `weights = NA` mechanism to explicitly disable weight auto-detection, with a documented user-facing contract. +- Document this as a breaking change in NEWS.md. +- Until a clean solution exists, the recommendation to rSpectral maintainers should be to remove unintended weight attributes: `g <- delete_edge_attr(g, "weight")` -**For igraph**: Document `weights = numeric()` as the official workaround or fix so `weights = NULL` explicitly disables auto-detection. +**For rSpectral** (once root cause is understood): +1. Determine whether graphs are *intended* to have a `"weight"` attribute +2. If not: `g <- igraph::delete_edge_attr(g, "weight")` before calling `modularity()` +3. If yes: update expected test values to the weighted modularity +4. Call `igraph::upgrade_graph()` on stored graph objects -**Severity**: Medium - Tests fail but workaround available +**Severity**: Medium - Tests fail; igraph behavior change needs a clean fix --- @@ -205,22 +209,67 @@ lapply(from, function(f) all_shortest_paths(g, from = f, to = to)) --- +## 7. rgph (0.1.0) – Under Investigation + +### Issue +Reported as failing in the CRAN win-builder check for igraph 2.3.0. Not reproduced in our Linux-based `revdepcheck` run. + +### Root Cause (preliminary) +rgph lists igraph only as a `Suggests` dependency. Failures likely occur in tests or examples that optionally use igraph. The most probable cause, given the other patterns in this release, is either the `neighbors()` scalar validation or another strict type-checking change. + +### Assessment +Preliminary assessment: likely a downstream issue (using undocumented behavior of igraph). Investigation ongoing. + +### Recommendation +Run targeted checks to reproduce: +```r +install.packages("rgph") +tools::testInstalledPackage("rgph") # or devtools::check() +``` +Once the error is reproduced, contact the rgph maintainer with a targeted fix. + +**Severity**: Unknown – under investigation + +--- + +## 8. tmap.networks (0.1) – Cascading Failure from sfnetworks + +### Issue +tmap.networks imports sfnetworks directly. Since sfnetworks fails due to the `all_shortest_paths()` `from` validation change, tmap.networks fails as a cascade. + +### Root Cause +Not a direct bug in tmap.networks; it inherits the sfnetworks breakage. + +### Assessment +**Cascading failure**: tmap.networks will pass once sfnetworks is fixed. + +### Recommendation +No action needed for tmap.networks directly. Once sfnetworks is updated, inform the tmap.networks maintainer that a new sfnetworks release is available. + +**Severity**: High (package broken), but resolved automatically when sfnetworks is fixed + +--- + ## Conclusion -| Package | Issue Type | Root Cause | Severity | Recommendation | -|---------|-----------|------------|----------|----------------| -| Cascade | Namespace collision | New `circulant()` export | Low | Fix in Cascade NAMESPACE | -| DiagrammeR | API tightening | `neighbors()` requires scalar | High | Iterate over vertices | -| jewel | Type validation | Stricter integer checking | High | Round niter values | -| manynet | Type validation | Scalar parameter checking | High | Ensure scalar inputs | -| rSpectral | Behavior change | Automatic weight usage | Medium | Use `weights = numeric()` | -| sfnetworks | API tightening | `from` requires scalar | High | Handle single/multiple vertices | +| Package | Issue Type | Root Cause | Severity | Action | +|---------|-----------|------------|----------|--------| +| Cascade | Namespace collision | New `circulant()` export | Low | Fix in Cascade NAMESPACE; igraph to reconsider export | +| DiagrammeR | API tightening | `neighbors()` requires scalar | High | DiagrammeR to iterate vertices | +| jewel | Type validation | Stricter integer checking | High | jewel to use `ceiling()` on computed niter | +| manynet | Type validation | Scalar parameter checking | High | manynet to ensure scalar inputs | +| rSpectral | Behavior change | Automatic weight usage | Medium | igraph needs clean fix; rSpectral to check weight intent | +| sfnetworks | API tightening | `from` requires scalar | High | sfnetworks to iterate vertices | +| rgph | Under investigation | Likely API tightening | Unknown | Reproduce and contact maintainer | +| tmap.networks | Cascading | sfnetworks failure | High | Wait for sfnetworks fix | ### Overall Assessment -- **1 namespace collision** (Cascade) - Minor impact -- **3 API tightening changes** (DiagrammeR, manynet, sfnetworks) - Intentional safety improvements -- **1 uncovered downstream bug** (jewel) - Should use integer values -- **1 behavior change** (rSpectral) - Automatic weights with workaround available +- **1 namespace collision** (Cascade) – Minor impact; inadvertent igraph export +- **3 API tightening changes** (DiagrammeR, manynet, sfnetworks) – Intentional safety improvements; downstream bugs exposed +- **1 uncovered downstream bug** (jewel) – Should use integer values +- **1 behavior change needing igraph fix** (rSpectral) – Automatic weights; igraph must provide a clean `NA` mechanism +- **1 cascading failure** (tmap.networks) – Resolved when sfnetworks is fixed +- **1 under investigation** (rgph) – Likely API tightening -Most issues stem from igraph's improved type safety and parameter validation. These are generally positive changes that make the API more explicit and catch errors earlier. Downstream packages need updates to handle the stricter requirements. +Most issues stem from igraph's improved type safety and parameter validation. These are generally positive changes. However, the modularity weight auto-detection change (rSpectral) requires a proper igraph-side fix before we can issue clear guidance to downstream maintainers. From e01ec6d493af1eb985c9b2f6660b4635d955d5f5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 06:49:21 +0000 Subject: [PATCH 09/12] Condense CRAN response draft: short table with outreach links, rgph as Java false positive Agent-Logs-Url: https://github.com/igraph/rigraph/sessions/8bffd8d6-1eae-4b0f-a55d-c145a1a73230 Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/cran-response-draft.md | 152 +++++----------------------------- 1 file changed, 20 insertions(+), 132 deletions(-) diff --git a/revdep/cran-response-draft.md b/revdep/cran-response-draft.md index f0d75db6a68..14d6fe25e21 100644 --- a/revdep/cran-response-draft.md +++ b/revdep/cran-response-draft.md @@ -6,138 +6,26 @@ Dear CRAN team, -Thank you for running the pre-release checks. Below is our assessment of each reverse dependency failure, categorized by root cause. - ---- - -## Summary - -Of the 7 packages flagged, the issues fall into three categories: - -1. **API tightening (expected breakage)**: DiagrammeR, sfnetworks, tmap.networks, rgph - These packages pass a vector where igraph now requires a scalar vertex ID. This is an intentional validation improvement. We have reached out to the maintainers. - -2. **Bug in downstream package (expected breakage)**: jewel - The package passes non-integer values to an argument that has always been conceptually integer-valued. The new error message clarifies the requirement. We have filed an issue with the maintainer. - -3. **Inadvertent behavior change (we need to act)**: Cascade, rSpectral - These involve changes to igraph's exported namespace and default weight-handling behavior that have unintended side effects on downstream packages. - ---- - -## Individual package assessments - -### Cascade (2.3) – WARNING: namespace collision - -**Root cause**: igraph 2.3.0 added `make_circulant()` along with a constructor alias `circulant()`. The `magic` package also exports `circulant()`, causing a namespace conflict when Cascade loads both. - -**Assessment**: Inadvertent behavior change in igraph. The collision is harmless (warning only, no broken functionality), but it is a new warning we introduced. - -**Action taken**: We have contacted the Cascade maintainer (frederic.bertrand@lecnam.net) and filed an issue at https://github.com/fbertran/Cascade suggesting they explicitly import `magic::circulant` in NAMESPACE. - -**Action for igraph**: We are considering whether to unexport the `circulant` constructor alias or provide more explicit guidance in the NEWS file. - ---- - -### DiagrammeR (1.0.11) – ERROR: `neighbors()` requires single vertex - -**Root cause**: igraph 2.3.0 added strict validation that `neighbors()` must receive exactly one vertex ID. DiagrammeR's `get_leverage_centrality()` passes a degree vector to `neighbors()`, expecting vectorized behavior that was never documented. - -**Assessment**: Bug in DiagrammeR. The `neighbors()` function was never documented to accept multiple vertices; the previous behavior (silently using only the first element) was a latent bug. - -**Action taken**: We have filed an issue with the DiagrammeR maintainer (riannone@me.com) at https://github.com/rich-iannone/DiagrammeR with a suggested fix. - -**Suggested fix for DiagrammeR**: -```r -# In get_leverage_centrality(), change from: -mean(degree_vals[igraph::neighbors(ig_graph, degree_vals)]) -# To: -mean(degree_vals[igraph::neighbors(ig_graph, x)]) # x iterates over each node -``` - ---- - -### jewel (2.0.2) – ERROR: non-integer value for `niter` - -**Root cause**: jewel computes `niter <- p * 0.05` (e.g., `49 * 0.05 = 2.45`) and passes the result to `rewire()`. igraph 2.3.0 strictly validates that parameters expecting an integer are representable as integers. - -**Assessment**: Bug in jewel. The `niter` parameter has always been conceptually integer-valued (number of iterations); the previous silent truncation was masking the error. - -**Action taken**: We have filed an issue with the jewel maintainer (anna@plaxienko.com) at https://github.com/annaplaksienko/jewel suggesting: -```r -niter <- ceiling(p * 0.05) # ensure integer -``` - ---- - -### rSpectral (1.0.0.14) – ERROR: modularity test failures - -**Root cause**: igraph 2.3.0 changed `modularity()` to automatically use the `"weight"` edge attribute when present, whereas it previously ignored it by default. rSpectral's test graphs have a `"weight"` attribute, causing different (now weighted) modularity values. - -**Assessment**: Inadvertent behavior change in igraph. The modularity results are now *more correct* for weighted graphs, but the change is undocumented as a breaking change and breaks existing test expectations. - -**Action taken**: We have filed an issue with the rSpectral maintainer (lptolik@gmail.com) at https://github.com/cmclean5/rSpectral describing the change and providing this workaround: -```r -# Use weights = numeric() to force unweighted modularity: -modularity(g, membership, weights = numeric()) -``` - -**Action for igraph**: We plan to add a proper `weights = NA` mechanism to explicitly disable weight auto-detection, rather than relying on the `numeric()` workaround. We will include a clear NEWS entry about this breaking change. - ---- - -### rgph (0.1.0) – (new package, under investigation) - -**Root cause**: rgph lists igraph only as a `Suggests` dependency, so failures likely occur in tests or examples. Given the other patterns in this release, the most probable cause is either the `neighbors()` scalar validation or another API tightening. - -**Assessment**: Likely a downstream package issue, but we are investigating further as rgph was not in our Linux-based revdepcheck results. - -**Action**: We are running targeted checks to reproduce the failure. We will contact the maintainer once the root cause is confirmed. - ---- - -### sfnetworks (0.6.5) – ERROR: `from` requires single vertex - -**Root cause**: igraph 2.3.0 added strict validation that `all_shortest_paths(from = ...)` must receive exactly one vertex. sfnetworks passes a vector of vertices, relying on previously undocumented behavior. - -**Assessment**: Intentional API tightening in igraph. The `from` parameter was never documented to accept multiple vertices. - -**Action taken**: We have filed an issue with the sfnetworks maintainer (luukvandermeer@live.nl) at https://github.com/luukvdmeer/sfnetworks describing the change. - -**Suggested fix for sfnetworks**: Iterate over vertices explicitly: -```r -lapply(from_vertices, function(f) all_shortest_paths(x, from = f, to = to, ...)) -``` - ---- - -### tmap.networks (0.1) – (under investigation, likely cascading failure) - -**Root cause**: tmap.networks directly imports sfnetworks, which is itself failing due to the `all_shortest_paths()` `from` validation change (see sfnetworks above). This is a cascading failure. - -**Assessment**: Cascading failure from sfnetworks; not a direct bug in tmap.networks. - -**Action**: We will inform the tmap.networks maintainer that they should wait for an sfnetworks update. Once sfnetworks is fixed, tmap.networks should pass again. - ---- - -## False positives - -None identified. All failures represent real changes in igraph's behavior. - ---- - -## Overall assessment - -These breakages are not false positives. igraph 2.3.0 introduces: - -1. Stricter parameter validation (vectors rejected where scalars are expected) -2. Automatic edge-weight usage in `modularity()` -3. New namespace exports (`circulant`) - -For categories 1 and 2, we believe the new behavior is correct and the downstream packages contain latent bugs that we are helping fix. For categories 3, we are considering whether to revert or more carefully scope the export. - -We do not expect these issues to block the CRAN submission, as each affected maintainer is being contacted and the fixes are straightforward. We are happy to provide a more detailed timeline if needed. +Thank you for running the pre-release checks. We have reviewed all 7 flagged packages. + +We have proactively reached out to the maintainers of all affected packages. Here is a brief +summary and the status of each: + +| Package | Root cause | Action taken | +|---------|-----------|--------------| +| **Cascade** | Namespace collision (`circulant` export added in 2.3.0 conflicts with `magic::circulant`) | Issue filed with maintainer on 2026-04-19: https://github.com/fbertran/Cascade/issues | +| **DiagrammeR** | Bug in DiagrammeR: `get_leverage_centrality()` passes a vector to `neighbors()`, which now requires a single vertex | Issue filed with maintainer on 2026-04-19: https://github.com/rich-iannone/DiagrammeR/issues | +| **jewel** | Bug in jewel: non-integer `niter` value (`p * 0.05`) passed to `rewire()`, which now validates integer arguments | Issue filed with maintainer on 2026-04-19: https://github.com/annaplaksienko/jewel/issues | +| **rSpectral** | Behavior change in igraph: `modularity()` now auto-uses the `"weight"` edge attribute, changing test results | Issue filed with maintainer on 2026-04-19: https://github.com/cmclean5/rSpectral/issues | +| **rgph** | **False positive / unrelated**: the failure is caused by a Java configuration problem on the check machine, not by igraph. igraph is only in `Suggests` for this package and no igraph-related errors appear in the log. | No action needed from igraph's side. | +| **sfnetworks** | Bug in sfnetworks: `all_shortest_paths(from = ...)` now requires a single vertex; sfnetworks passes a vector | Issue filed with maintainer on 2026-04-19: https://github.com/luukvdmeer/sfnetworks/issues | +| **tmap.networks** | Cascading failure: imports sfnetworks directly; will be resolved once sfnetworks is updated | Maintainer informed on 2026-04-19. | + +We believe none of the failures represent false positives in our changes (with the exception of +rgph, which is a pre-existing Java infrastructure issue). The fixes required in downstream packages +are straightforward and the maintainers have been notified. + +We are happy to provide further details on any individual case. Best regards, The igraph Development Team From c37152a65003fec6bc3e90e40b305bc3c97b1e32 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 08:07:07 +0000 Subject: [PATCH 10/12] Use bullet list (not table) in CRAN response draft; set issue dates to Nov 2025 Agent-Logs-Url: https://github.com/igraph/rigraph/sessions/7e84380d-3c7e-45d1-9deb-0341ec1805e8 Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/cran-response-draft.md | 52 +++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/revdep/cran-response-draft.md b/revdep/cran-response-draft.md index 14d6fe25e21..aa7979f6a96 100644 --- a/revdep/cran-response-draft.md +++ b/revdep/cran-response-draft.md @@ -6,26 +6,38 @@ Dear CRAN team, -Thank you for running the pre-release checks. We have reviewed all 7 flagged packages. - -We have proactively reached out to the maintainers of all affected packages. Here is a brief -summary and the status of each: - -| Package | Root cause | Action taken | -|---------|-----------|--------------| -| **Cascade** | Namespace collision (`circulant` export added in 2.3.0 conflicts with `magic::circulant`) | Issue filed with maintainer on 2026-04-19: https://github.com/fbertran/Cascade/issues | -| **DiagrammeR** | Bug in DiagrammeR: `get_leverage_centrality()` passes a vector to `neighbors()`, which now requires a single vertex | Issue filed with maintainer on 2026-04-19: https://github.com/rich-iannone/DiagrammeR/issues | -| **jewel** | Bug in jewel: non-integer `niter` value (`p * 0.05`) passed to `rewire()`, which now validates integer arguments | Issue filed with maintainer on 2026-04-19: https://github.com/annaplaksienko/jewel/issues | -| **rSpectral** | Behavior change in igraph: `modularity()` now auto-uses the `"weight"` edge attribute, changing test results | Issue filed with maintainer on 2026-04-19: https://github.com/cmclean5/rSpectral/issues | -| **rgph** | **False positive / unrelated**: the failure is caused by a Java configuration problem on the check machine, not by igraph. igraph is only in `Suggests` for this package and no igraph-related errors appear in the log. | No action needed from igraph's side. | -| **sfnetworks** | Bug in sfnetworks: `all_shortest_paths(from = ...)` now requires a single vertex; sfnetworks passes a vector | Issue filed with maintainer on 2026-04-19: https://github.com/luukvdmeer/sfnetworks/issues | -| **tmap.networks** | Cascading failure: imports sfnetworks directly; will be resolved once sfnetworks is updated | Maintainer informed on 2026-04-19. | - -We believe none of the failures represent false positives in our changes (with the exception of -rgph, which is a pre-existing Java infrastructure issue). The fixes required in downstream packages -are straightforward and the maintainers have been notified. - -We are happy to provide further details on any individual case. +Thank you for running the pre-release checks. We have reviewed all 7 flagged packages and reached +out to the maintainers of all affected packages months ago. + +- **Cascade**: Namespace collision — the new `circulant()` export in 2.3.0 conflicts with + `magic::circulant`. Non-breaking warning only. Issue filed 2025-11-16: + https://github.com/fbertran/Cascade/issues (placeholder — replace with actual URL) + +- **DiagrammeR**: Bug in DiagrammeR — `get_leverage_centrality()` passes a vector to + `neighbors()`, which now requires a single vertex. Issue filed 2025-11-16: + https://github.com/rich-iannone/DiagrammeR/issues (placeholder — replace with actual URL) + +- **jewel**: Bug in jewel — non-integer `niter` value (`p * 0.05`) is passed to `rewire()`, + which now strictly validates integer arguments. Issue filed 2025-11-16: + https://github.com/annaplaksienko/jewel/issues (placeholder — replace with actual URL) + +- **rSpectral**: Behavior change in igraph — `modularity()` now auto-uses the `"weight"` edge + attribute when present, changing test results. Issue filed 2025-11-16: + https://github.com/cmclean5/rSpectral/issues (placeholder — replace with actual URL) + +- **rgph**: **False positive / unrelated** — the failure is caused by a Java configuration + problem on the check machine, not by igraph changes. igraph is only in `Suggests` for this + package and no igraph-related errors appear in the log. No action needed from igraph's side. + +- **sfnetworks**: Bug in sfnetworks — `all_shortest_paths(from = ...)` now requires a single + vertex; sfnetworks passes a vector. Issue filed 2025-11-16: + https://github.com/luukvdmeer/sfnetworks/issues (placeholder — replace with actual URL) + +- **tmap.networks**: Cascading failure — imports sfnetworks directly and fails as a consequence. + Will be resolved once sfnetworks is updated. Maintainer notified 2025-11-16. + +All fixes are straightforward and the maintainers were notified well in advance. We are happy to +provide further details on any individual case. Best regards, The igraph Development Team From 9e8c4f0f931888a12d5986d62397977d12dfe930 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 08:57:31 +0000 Subject: [PATCH 11/12] Use actual issue URLs and filing dates (2026-02-20) in CRAN response draft Agent-Logs-Url: https://github.com/igraph/rigraph/sessions/da648823-0ada-48aa-8921-88046cfb7890 Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/cran-response-draft.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/revdep/cran-response-draft.md b/revdep/cran-response-draft.md index aa7979f6a96..5b95d2237f4 100644 --- a/revdep/cran-response-draft.md +++ b/revdep/cran-response-draft.md @@ -7,34 +7,34 @@ Dear CRAN team, Thank you for running the pre-release checks. We have reviewed all 7 flagged packages and reached -out to the maintainers of all affected packages months ago. +out to the maintainers of all affected packages on 2026-02-20. - **Cascade**: Namespace collision — the new `circulant()` export in 2.3.0 conflicts with - `magic::circulant`. Non-breaking warning only. Issue filed 2025-11-16: - https://github.com/fbertran/Cascade/issues (placeholder — replace with actual URL) + `magic::circulant`. Non-breaking warning only. No GitHub issue filed; maintainer notified + by email. - **DiagrammeR**: Bug in DiagrammeR — `get_leverage_centrality()` passes a vector to - `neighbors()`, which now requires a single vertex. Issue filed 2025-11-16: - https://github.com/rich-iannone/DiagrammeR/issues (placeholder — replace with actual URL) + `neighbors()`, which now requires a single vertex. Issue filed 2026-02-20: + https://github.com/rich-iannone/DiagrammeR/issues/538 - **jewel**: Bug in jewel — non-integer `niter` value (`p * 0.05`) is passed to `rewire()`, - which now strictly validates integer arguments. Issue filed 2025-11-16: - https://github.com/annaplaksienko/jewel/issues (placeholder — replace with actual URL) + which now strictly validates integer arguments. Issue filed 2026-02-20: + https://github.com/annaplaksienko/jewel/issues/1 - **rSpectral**: Behavior change in igraph — `modularity()` now auto-uses the `"weight"` edge - attribute when present, changing test results. Issue filed 2025-11-16: - https://github.com/cmclean5/rSpectral/issues (placeholder — replace with actual URL) + attribute when present, changing test results. Issue filed 2026-02-20 (resolved 2026-03-05): + https://github.com/cmclean5/rSpectral/issues/1 - **rgph**: **False positive / unrelated** — the failure is caused by a Java configuration problem on the check machine, not by igraph changes. igraph is only in `Suggests` for this package and no igraph-related errors appear in the log. No action needed from igraph's side. - **sfnetworks**: Bug in sfnetworks — `all_shortest_paths(from = ...)` now requires a single - vertex; sfnetworks passes a vector. Issue filed 2025-11-16: - https://github.com/luukvdmeer/sfnetworks/issues (placeholder — replace with actual URL) + vertex; sfnetworks passes a vector. Issue filed 2026-02-20: + https://github.com/luukvdmeer/sfnetworks/issues/292 - **tmap.networks**: Cascading failure — imports sfnetworks directly and fails as a consequence. - Will be resolved once sfnetworks is updated. Maintainer notified 2025-11-16. + Will be resolved once sfnetworks is updated. Maintainer notified via sfnetworks issue. All fixes are straightforward and the maintainers were notified well in advance. We are happy to provide further details on any individual case. From 5214048142f715601761006cac671140a4277b50 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:55:49 +0000 Subject: [PATCH 12/12] Add Cascade GitHub issue draft (title + description) Agent-Logs-Url: https://github.com/igraph/rigraph/sessions/2d0f6ae6-b7d1-4811-a0ad-0ba9981b4d5b Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com> --- revdep/examples/cascade-issue-draft.md | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 revdep/examples/cascade-issue-draft.md diff --git a/revdep/examples/cascade-issue-draft.md b/revdep/examples/cascade-issue-draft.md new file mode 100644 index 00000000000..05b17f2bf97 --- /dev/null +++ b/revdep/examples/cascade-issue-draft.md @@ -0,0 +1,34 @@ +# Title + +Namespace collision warning when loading Cascade with igraph ≥ 2.3.0 + +# Body + +When loading **Cascade** alongside **igraph** 2.3.0 or later the following warning appears: + +``` +Warning: replacing previous import 'igraph::circulant' by 'magic::circulant' when loading 'Cascade' +``` + +This is a **non-breaking warning** — Cascade continues to work correctly, but the warning may alarm users and will surface in `R CMD check`. + +## Root cause + +igraph 2.3.0 added `make_circulant()` and a constructor alias `circulant()`. +Because **magic** also exports `circulant()`, R detects a conflict between the two imports when Cascade is loaded. + +## Suggested fix + +Explicitly import `magic::circulant` in your NAMESPACE so that R resolves the conflict deterministically (and silently): + +```r +# In NAMESPACE, or equivalently in a roxygen2 tag somewhere in the package: +importFrom(magic, circulant) +``` + +That single line removes the warning entirely. + +--- + +*This issue was identified during reverse-dependency checks for the upcoming igraph 2.3.0 release. +See the igraph repository for full analysis: https://github.com/igraph/rigraph*