diff --git a/revdep/.gitignore b/revdep/.gitignore index a2b0d9bb97..b06d4d8201 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 0000000000..bf219ed9d6 --- /dev/null +++ b/revdep/NOTIFY-README.md @@ -0,0 +1,131 @@ +# 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): + - 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 + +Files are created in the `notifications/` directory **only for packages that require email drafts**: + +- `{Package}-email.txt` - Complete email draft with subject and body + +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 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` + +### 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. 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 + +### 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/examples/README.md b/revdep/examples/README.md new file mode 100644 index 0000000000..510d4d899e --- /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/examples/cascade-circulant-issue.R b/revdep/examples/cascade-circulant-issue.R new file mode 100644 index 0000000000..bc6d4a6902 --- /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 0000000000..e1c721d7ce --- /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 0000000000..ca1c92cf12 --- /dev/null +++ b/revdep/examples/rspectral-modularity-issue.R @@ -0,0 +1,75 @@ +#!/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("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") +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") diff --git a/revdep/notify-maintainers.sh b/revdep/notify-maintainers.sh new file mode 100755 index 0000000000..de02148747 --- /dev/null +++ b/revdep/notify-maintainers.sh @@ -0,0 +1,329 @@ +#!/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 GitHub issue +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) + 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 create an email draft +create_email_draft() { + local package=$1 + 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 +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 +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: + +``` +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' + +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 "" + +# 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: + +``` +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' + +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" + +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: + +``` +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"' + +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 "" + +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)" +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 new file mode 100644 index 0000000000..03422811a2 --- /dev/null +++ b/revdep/problems-analysis.md @@ -0,0 +1,236 @@ +# 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. + +**Note**: Runnable R scripts demonstrating each issue can be found in the `examples/` directory. + +## 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, 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 +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. + +**However**, passing `weights = numeric()` provides a simple workaround to force unweighted calculations. + +### 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 +``` + +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 + +### 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 + +| 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