Skip to content

Commit 8fc3c50

Browse files
committed
updated with tk plot
1 parent badc264 commit 8fc3c50

8 files changed

Lines changed: 386 additions & 3 deletions

‎revdep/examples/README.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ Each issue has two files:
3232
#### Group E — Indirect path through a defunct fallback
3333
12. **degreenet-reedmolloy-issue** — defunct `sample_degseq(method = "simple.no.multiple")` fallback inside `reedmolloy()`'s retry loop
3434

35+
#### Group F — `tkplot()` defunct (igraph 3.0.0)
36+
13. **tkplot-defunct-issue** — shared reproducer for Boptbd, c3net, ggm, optbdmaeAT, optrcdmaeAT (all direct callers of `igraph::tkplot()`)
37+
38+
#### Group G — API tightening / behavioural changes
39+
14. **jewel-integer-issue** — strict integer validation in `rewire_impl()`
40+
15. **rspectral-modularity-issue** — `modularity()` auto-uses `"weight"` edge attribute
41+
3542
Three packages from the failure set (dci, ggraph, scistreer) are pure cascades through tidygraph and have no separate reproducer — see `tidygraph-bfs-father-issue` for the root cause.
3643

3744
## Running the Examples
@@ -77,5 +84,12 @@ The examples follow a simplified format:
7784
| R6causal | Install-time deprecation warnings | Medium | Pre-emptive rename needed |
7885
| migraph | `create_ring(width=)` warning → test failure | Low | Deprecated argument |
7986
| degreenet | Defunct fallback `sample_degseq(method=)` value | High | Defunct argument value (indirect) |
87+
| Boptbd | `tkplot()` defunct | High | Removed function (PR #2615) |
88+
| c3net | `tkplot()` defunct | High | Removed function (PR #2615) |
89+
| ggm | `tkplot()` defunct | High | Removed function (PR #2615) |
90+
| optbdmaeAT | `tkplot()` defunct | High | Removed function (PR #2615) |
91+
| optrcdmaeAT | `tkplot()` defunct | High | Removed function (PR #2615) |
92+
| jewel | Strict integer validation in `rewire_impl()` | High | API tightening |
93+
| rSpectral | `modularity()` auto-uses `"weight"` | Medium | Behavioural change (needs igraph opt-out) |
8094

8195
See [`../problems-analysis.md`](../problems-analysis.md) for detailed analysis and recommendations.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# jewel non-integer rewire iteration count
2+
# Issue: jewel computes niter = p * 0.05 (fractional) and passes it through
3+
# igraph::rewire(), whose C interface now strictly rejects non-integer values.
4+
5+
library(igraph)
6+
7+
g <- make_ring(20)
8+
9+
# Old call pattern -- niter ends up fractional
10+
p <- 49
11+
niter_bad <- p * 0.05 # 2.45
12+
13+
# rewire(g, with = keeping_degseq(niter = niter_bad))
14+
# Error in rewire_impl(rewire = graph, n = niter, mode = mode) :
15+
# The value 2.45 is not representable as an integer. Invalid value
16+
# Source: rinterface_extra.c:83
17+
18+
# Working replacement -- round to an integer up front
19+
niter <- as.integer(ceiling(p * 0.05))
20+
res <- rewire(g, with = keeping_degseq(niter = niter))
21+
ecount(res)
22+
23+
# Root cause:
24+
# - igraph's C interface now strictly validates that numeric arguments are
25+
# exactly representable as integers. Fractional values were previously
26+
# truncated silently.
27+
# - jewel:::generateData_rewire() ultimately calls rewire() with
28+
# `niter = p * 0.05`, which is non-integer for many p.
29+
30+
# Assessment:
31+
# - Bug in jewel -- a count of iterations should always be an integer.
32+
33+
# Recommendation:
34+
# - For jewel: integer-round the value before the rewire() call, e.g.
35+
# niter <- as.integer(ceiling(p * 0.05))
36+
# (or floor(), or round() -- whichever matches the intended semantics).
37+
# - For igraph (optional defensive measure): consider adding as.integer()
38+
# on the R side in rewire_keeping_degseq() before calling rewire_impl().
39+
# The strict C-side check is the correct contract.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# jewel non-integer rewire iteration count
2+
3+
## Issue
4+
```
5+
Error in rewire_impl(rewire = graph, n = niter, mode = mode) :
6+
The value 2.4500000000000002 is not representable as an integer. Invalid value
7+
Source: rinterface_extra.c:83
8+
```
9+
10+
`jewel:::generateData_rewire()` computes `niter = p * 0.05` (often fractional) and passes it through `igraph::rewire()`, whose C interface now strictly rejects non-integer values.
11+
12+
## Reproducible Example
13+
14+
```r
15+
library(igraph)
16+
g <- make_ring(20)
17+
p <- 49
18+
niter_bad <- p * 0.05 # 2.45
19+
20+
rewire(g, with = keeping_degseq(niter = niter_bad))
21+
#> Error in rewire_impl(rewire = graph, n = niter, mode = mode) :
22+
#> The value 2.45 is not representable as an integer. Invalid value
23+
24+
# Working replacement
25+
rewire(g, with = keeping_degseq(niter = as.integer(ceiling(p * 0.05))))
26+
```
27+
28+
## Root Cause
29+
igraph's C interface now strictly validates that numeric arguments are exactly representable as integers. Previously, fractional values were silently truncated.
30+
31+
## Assessment
32+
Bug in jewel — a count of iterations should always be an integer.
33+
34+
## Recommendation
35+
**For jewel**: integer-round the value before the `rewire()` call: `niter <- as.integer(ceiling(p * 0.05))` (or `floor()`, or `round()` — pick the one that matches intended semantics).
36+
37+
**For igraph** (optional defensive measure): consider adding `as.integer()` on the R side in `rewire_keeping_degseq()` before calling `rewire_impl()`. The strict C-side check is the correct contract, but the R wrapper could be friendlier.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# rSpectral modularity() now auto-uses the "weight" edge attribute
2+
# Issue: rSpectral's stored test graphs carry a "weight" attribute. Previously
3+
# modularity() ignored it unless weights = E(g)$weight was passed explicitly;
4+
# now modularity() auto-detects and uses the attribute, changing the computed
5+
# values and breaking baseline assertions.
6+
7+
library(igraph)
8+
9+
# A small graph with a weight attribute
10+
g <- make_ring(5)
11+
E(g)$weight <- c(1, 2, 3, 4, 5)
12+
13+
# Same membership, two different modularity calls
14+
membership <- c(1, 1, 2, 2, 2)
15+
16+
# Old behaviour (ignored weights unless passed explicitly):
17+
# the value below is what modularity() returned before the change
18+
modularity(g, membership, weights = rep(1, ecount(g)))
19+
20+
# New behaviour (auto-detects $weight):
21+
modularity(g, membership)
22+
23+
# These two now differ -- which is exactly what breaks rSpectral's tests.
24+
25+
# Root cause:
26+
# - Around igraph 2.2.1.9004, modularity() was changed to use the "weight"
27+
# edge attribute automatically when present:
28+
# if (is.null(weights) && "weight" %in% edge_attr_names(graph)) {
29+
# weights <- E(graph)$weight
30+
# }
31+
# - rSpectral's stored test graphs carry a "weight" attribute that was not
32+
# intended to be consumed by modularity().
33+
34+
# Assessment:
35+
# - Behavioural change in igraph. Inadvertent semantics shift for callers
36+
# that did not intend to use weights. There is currently no clean
37+
# `weights = NA` opt-out.
38+
39+
# Recommendation:
40+
# - For igraph (action required before notifying rSpectral): provide a clean
41+
# opt-out -- e.g. accept `weights = NA` to disable auto-detection -- and
42+
# document the contract change in NEWS.md.
43+
# - For rSpectral (once an opt-out exists, or after igraph adjusts):
44+
# 1. Determine whether the stored graphs were intended to carry $weight.
45+
# 2. If not: g <- igraph::delete_edge_attr(g, "weight") before modularity()
46+
# and run igraph::upgrade_graph() on stored graph objects.
47+
# 3. If yes: update the expected modularity baselines.
48+
49+
# Workaround that happens to disable auto-detection today (DO NOT recommend
50+
# to downstream maintainers -- relies on implementation detail):
51+
# modularity(g, membership, weights = numeric())
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# rSpectral modularity() now auto-uses the "weight" edge attribute
2+
3+
## Issue
4+
Two test failures: modularity values diverge from the expected baselines.
5+
6+
```
7+
── Failure ('test-igraph.R:19:3'): membership fix_neig=0 is correct ────────────
8+
Expected `c$modularity` to equal `exp_mod10`.
9+
`actual`: 0.432
10+
`expected`: 0.408
11+
12+
── Failure ('test-igraph.R:26:3'): membership fix_neig=1 is correct ────────────
13+
`actual`: 0.3758
14+
`expected`: 0.3776
15+
```
16+
17+
## Reproducible Example
18+
19+
```r
20+
library(igraph)
21+
g <- make_ring(5)
22+
E(g)$weight <- c(1, 2, 3, 4, 5)
23+
membership <- c(1, 1, 2, 2, 2)
24+
25+
# Old behaviour (ignored $weight unless passed explicitly)
26+
modularity(g, membership, weights = rep(1, ecount(g)))
27+
# New behaviour (auto-detects $weight)
28+
modularity(g, membership)
29+
# The two now differ.
30+
```
31+
32+
## Root Cause
33+
Around igraph 2.2.1.9004, `modularity()` was changed to use the `"weight"` edge attribute automatically when present:
34+
```r
35+
if (is.null(weights) && "weight" %in% edge_attr_names(graph)) {
36+
weights <- E(graph)$weight
37+
}
38+
```
39+
rSpectral's stored test graphs carry a `"weight"` attribute that was not intended to be consumed by `modularity()`.
40+
41+
## Assessment
42+
Behavioural change in igraph. Inadvertent semantics shift for callers that did not intend to use weights. No clean `weights = NA` opt-out currently exists.
43+
44+
## Recommendation
45+
**For igraph** (action required before notifying rSpectral): provide a clean opt-out — e.g. accept `weights = NA` to disable auto-detection — and document the contract change in `NEWS.md`.
46+
47+
**For rSpectral** (once an opt-out exists, or after igraph adjusts):
48+
1. Determine whether the stored graphs were intended to carry `$weight`.
49+
2. If not: `g <- igraph::delete_edge_attr(g, "weight")` before `modularity()`, and run `igraph::upgrade_graph()` on the stored graph objects.
50+
3. If yes: update the expected modularity baselines.
51+
52+
A hacky workaround that happens to disable auto-detection today is `modularity(g, membership, weights = numeric())`, but this relies on internal implementation detail and **must not** be recommended to downstream maintainers.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# tkplot() defunct (igraph 3.0.0)
2+
# Issue: tkplot() advanced to deprecate_stop in #2615. Five revdep packages
3+
# call it directly from their examples:
4+
# Boptbd::graphoptBbd()
5+
# c3net::netplot() (used by c3net::c3net(..., network = TRUE))
6+
# ggm::AG(..., plot = TRUE)
7+
# optbdmaeAT::graphoptbd.mae()
8+
# optrcdmaeAT::graphoptrcd.mae()
9+
10+
library(igraph)
11+
12+
g <- make_ring(5)
13+
14+
# Old call -- now defunct in igraph 3.0.0
15+
# tkplot(g)
16+
# Error:
17+
# ! `tkplot()` was deprecated in igraph 3.0.0 and is now defunct.
18+
19+
# Working replacement -- non-interactive plot
20+
plot(g)
21+
22+
# Root cause:
23+
# - The interactive Tk-based plotter has been removed; non-interactive plot()
24+
# is the recommended drop-in replacement.
25+
# - All five affected packages are direct callers.
26+
27+
# Assessment:
28+
# - Intentional removal in igraph (#2615).
29+
30+
# Recommendation:
31+
# - For each affected package: replace igraph::tkplot(g, ...) with
32+
# igraph::plot.igraph(g, ...) (or just plot(g, ...)).
33+
# - Tk-specific tuning functions (tkplot.canvas, tkplot.setcoords, ...) are
34+
# themselves deprecated and have no direct replacement. Packages that
35+
# genuinely needed interactive editing should adopt a JS-based plotter
36+
# (e.g. visNetwork) instead.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# tkplot() defunct (igraph 3.0.0)
2+
3+
## Issue
4+
`igraph::tkplot()` advanced to `deprecate_stop("3.0.0", "tkplot()")` in [#2615](https://github.com/igraph/rigraph/pull/2615). Five revdep packages call it directly from their examples and now hard-error with the same message.
5+
6+
| Package | Caller |
7+
|---|---|
8+
| Boptbd | `Boptbd::graphoptBbd()` |
9+
| c3net | `c3net::netplot()` (via `c3net::c3net(..., network = TRUE)`) |
10+
| ggm | `ggm::AG(..., plot = TRUE)` |
11+
| optbdmaeAT | `optbdmaeAT::graphoptbd.mae()` |
12+
| optrcdmaeAT | `optrcdmaeAT::graphoptrcd.mae()` |
13+
14+
## Reproducible Example
15+
16+
```r
17+
library(igraph)
18+
g <- make_ring(5)
19+
20+
tkplot(g)
21+
#> Error:
22+
#> ! `tkplot()` was deprecated in igraph 3.0.0 and is now defunct.
23+
24+
# Working replacement
25+
plot(g)
26+
```
27+
28+
## Root Cause
29+
The interactive Tk-based plotter has been removed. Non-interactive `plot()` (i.e. `plot.igraph()`) is the recommended drop-in replacement.
30+
31+
## Assessment
32+
Intentional removal in igraph (#2615). All five packages are direct callers; no cascading.
33+
34+
## Recommendation
35+
**For each affected package**: replace `igraph::tkplot(g, ...)` with `igraph::plot.igraph(g, ...)` (or just `plot(g, ...)`).
36+
37+
Tk-specific tuning functions (`tkplot.canvas`, `tkplot.setcoords`, …) are themselves deprecated and have no direct replacement. Packages that genuinely needed interactive graph editing should adopt a JS-based plotter (e.g. `visNetwork`) instead.

0 commit comments

Comments
 (0)