Fix edge.width not applied to drawn line widths in nplot#18
Merged
Conversation
Closed
Copilot
AI
changed the title
[WIP] Fix issue with edge width not being used in nplot function
Fix edge.width not applied to drawn line widths in nplot
Jun 30, 2026
gvegayon
marked this pull request as ready for review
June 30, 2026 01:37
Describe the actual fix (nplot.network now uses the weight attribute) and the related changes (NULL ranges, vertex.rot radian fix, edge.line.lty validation) instead of the inaccurate gpar-clobbering rationale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent handling of edge.width in nplot()—especially for network objects—so edge weights can correctly drive drawn line widths, and aligns scaling/rotation behaviors across the grid and base plotting implementations.
Changes:
- Make
edge.widthdefault to the"weight"edge attribute fornetworkobjects (and ensure widths are applied to the edge grobs after color post-processing). - Allow
edge.width.range,vertex.size.range, andvertex.label.rangeto beNULLto suppress rescaling and use values as-is. - Fix vertex rotation handling (preserve fractional radians) and correct named
"square"orientation; add/expand examples, tests, and documentation.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
R/netplot.R |
Updates nplot.network() defaults, adds NULL-range behavior, validates edge.line.lty, and defensively re-applies edge lwd after color edits. |
R/netplot_base.R |
Subsets edge parameters when sample.edges drops edges; supports NULL edge.width.range; applies vertex rotation to frame geometry. |
R/grob_vertex.R |
Fixes vertex rotation truncation; factors shape resolution into resolve_vertex_shape(); corrects "square" rotation. |
R/geometry.R |
Makes rescale_size() return input unchanged when rel = NULL. |
inst/tinytest/test_netplot.R |
Adds regression tests for edge width mapping, NULL ranges, vertex rotation, shape orientation, edge.line.lty validation, and sampling behavior. |
vignettes/examples.Rmd |
Adds an “Edge width” section and updates node-scaling example to demonstrate vertex.size.range = NULL. |
NEWS.md |
Documents the edge-width fix, NULL range behavior, vertex rotation fix, and edge.line.lty validation/sampling. |
man/nplot.Rd |
Updates parameter docs to describe new edge.width / edge.width.range semantics and vertex.size.range = NULL. |
man/nplot_base.Rd |
Mirrors updated parameter documentation for the base plotting variant. |
Makefile |
Simplifies developer targets and switches to devtools::document(), devtools::check(), and devtools::install(). |
DESCRIPTION |
Updates RoxygenNote. |
AGENTS.md |
Adds contributor/development workflow notes for agents and tooling. |
.Rbuildignore |
Excludes AGENTS.md and devcontainer/editor configuration from builds. |
.github/workflows/ci.yml |
Updates GitHub Actions versions used in CI. |
.github/workflows/website.yml |
Updates GitHub Actions versions used for the website workflow. |
.devcontainer/devcontainer.json |
Adds a devcontainer configuration for local development. |
.devcontainer/devcontainer-lock.json |
Locks devcontainer feature versions/digests. |
.devcontainer/Containerfile |
Defines the devcontainer image setup (R + Quarto + dependencies). |
.devcontainer/.vscode/settings.json |
Adds VS Code settings for the devcontainer environment. |
Files not reviewed (2)
- man/nplot.Rd: Generated file
- man/nplot_base.Rd: Generated file
Comments suppressed due to low confidence (1)
inst/tinytest/test_netplot.R:200
- Using
grDevices::pdf(NULL)will write anRplots.pdffile in the working directory. Prefer a temporary file to avoid leaving artifacts after tests run.
grDevices::pdf(NULL)
expect_error(
nplot_base(x, layout = l, skip.arrows = TRUE,
edge.line.lty = edge_ltys[seq_len(2)]),
"edge.line.lty",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+122
to
+132
| for (epar in edge_par) { | ||
| if (is.null(get(epar))) | ||
| next | ||
|
|
||
| if (is.matrix(get(epar)) || is.data.frame(get(epar))) { | ||
| if (nrow(get(epar)) == M) | ||
| assign(epar, get(epar)[sample.edges, , drop = FALSE]) | ||
| } else if (length(get(epar)) == M) { | ||
| assign(epar, get(epar)[sample.edges]) | ||
| } | ||
| } |
Comment on lines
+182
to
+190
| grDevices::pdf(NULL) | ||
| set.seed(1) | ||
| base_sample_lty <- tryCatch( | ||
| nplot_base(x_sample, layout = l_sample, skip.arrows = TRUE, | ||
| sample.edges = .4, edge.line.lty = sampled_ltys), | ||
| error = function(e) e | ||
| ) | ||
| grDevices::dev.off() | ||
|
|
- netplot_base(): cache each edge parameter once in the sample.edges block instead of calling get() repeatedly, so a (lazily-evaluated) argument is forced a single time before inspecting/subsetting it. - tests: point the throwaway graphics device at a tempfile instead of pdf(NULL) to make it explicit that no artifact is written. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thank you for your contribution @gvegayon 🚀! The pkgdown site preview is ready for review 👉 Download here 👈! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17 ("edge width is not used"). Edge widths were drawn uniformly regardless of edge weight for
networkobjects, and this PR makesedge.widthbehave consistently across methods, plus fixes a couple of related issues found along the way.Changes
R/netplot.R—nplot.network()now defaultsedge.widthto the"weight"edge attribute (matchingnplot.igraph()), falling back to1when absent. Previouslynetworkobjects ignored edge weights entirely and drew all edges at a single width — the actual cause of edge width is not used #17.R/netplot.R/R/netplot_base.R/R/geometry.R—edge.width.range,vertex.size.range, andvertex.label.rangenow acceptNULLto suppress rescaling and use the supplied values as-is (rescale_size()returns its input unchanged for aNULLrange).R/netplot.R— After color post-processing,edge.widthis explicitly re-applied viaset_edge_gpar(lwd = ...)for the"line"and"arrow"elements. This is defensive:grid::editGrob()merges graphical parameters, so the earlierset_edge_gpar(col = ...)did not actually droplwd, but re-applying keeps the drawn widths robust against future changes to the post-processing order.R/grob_vertex.R— Fixedvertex.rot, which was passed throughas.integer()and so silently ignored fractional (radian) rotations (e.g.pi/4became0). Shape resolution was factored intoresolve_vertex_shape(), and the named"square"orientation was corrected (pi/2→pi/4).R/netplot.R/R/netplot_base.R—edge.line.ltyis validated to be length 1 or one value per plotted edge, and edge parameters (includingedge.line.lty) are subset correctly whensample.edgesdrops edges.vignettes/examples.Rmd— Added an "Edge width" section demonstratingedge.width,edge.width.range(includingNULL), and post-hocset_edge_gpar(); updated the vertex-size example to showvertex.size.range = NULL.inst/tinytest/test_netplot.R— Added tests covering edge-width mapping andNULLranges,vertex.rotrotation, named-shape orientation,edge.line.ltyvalidation and sampling, and thenplot.networkdefault.NEWS.md— Changelog entries for the fixes.R/netplot.R,man/*) — Clarifiededge.width,edge.width.range, andvertex.size.rangebehavior, including theNULLoption and the range-mapping semantics.AGENTS.md.