Skip to content

Switch node fill aesthetic and replace Rgraphviz with improved Sugiyama layout#33

Closed
jhollway with Copilot wants to merge 9 commits into
mainfrom
copilot/update-fill-argument-in-ggplot
Closed

Switch node fill aesthetic and replace Rgraphviz with improved Sugiyama layout#33
jhollway with Copilot wants to merge 9 commits into
mainfrom
copilot/update-fill-argument-in-ggplot

Conversation

Copilot AI commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

Description

Shifts node colouring from color to fill aesthetic (closes #12), enabling independent control of node interior and border. Replaces the Rgraphviz Bioconductor dependency with a custom Sugiyama layout implementation that produces better edge crossing minimization than igraph::layout_with_sugiyama().

Node aesthetics: color → fill

  • geom_node_point() calls use fill instead of color/colour; scale functions updated accordingly
  • Default shape changed from "circle" to 21 (fillable circle); shape scale values use numeric codes (21–25) that support both fill (interior) and color (border)
  • Users can now layer borders: + geom_node_point(color = "black")

Sugiyama layout replacement

  • Removed Rgraphviz (Bioconductor) dependency entirely — replaced with .sugiyama_layout() in pure R
  • The new implementation inserts dummy nodes for multi-layer edges and runs iterative barycenter crossing minimization sweeps, which is what made Graphviz's dot layout superior to plain igraph::layout_with_sugiyama()
  • layout_hierarchy(), layout_alluvial(), and layout_lineage() all use the new engine
  • Removed BiocManager from Suggests, Rgraphviz from Enhances

Bug fix

  • graph_labels.R: Fixed case_when error from matrix subsetting ([,][[]])

Tests and coverage

  • Updated shape expectations ("circle"21) and scale checks (scale_color_*scale_fill_*)
  • Added demanding edge-crossing test: verifies optimised layout has strictly fewer crossings than naive ordering on ison_southern_women
  • Added fill aesthetic tests (fill mapping, multi-value scales, two-mode shapes, highlight palette)
  • Enabled hierarchy/two-mode tests on CI (removed skip_on_ci() since Rgraphviz no longer needed)
  • Added #nocov markers for untestable code (grapht.R, zzz.R, interactive helpers)

Checklist:

  • Documentation
    • DESCRIPTION file version is bumped by the appropriate increment (major, minor, patch)
    • Date in DESCRIPTION is correct
    • Longer functions are commented inline or broken down into helper functions to help debugging
  • PR form
    • Title indicates expected version number
    • PR description above and the NEWS.md file are aligned
    • Description above itemizes changes under subsection titles, e.g. "## Data"
    • Closed, fixed, or related issues are referenced and explained in the description above, e.g. "Fixed #0 by adding A"

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits February 19, 2026 08:05
Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>
Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>
Copilot AI changed the title [WIP] Update ggplot defaults to use fill instead of color Change node aesthetics from 'color' to 'fill' (closes #12) Feb 19, 2026
Copilot AI requested a review from jhollway February 19, 2026 08:10
@jhollway

Copy link
Copy Markdown
Contributor

@copilot , this doesn't pass the checks/tests. Is it the tests that need updating or does this break some other functionality?

Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>

Copilot AI commented Feb 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot , this doesn't pass the checks/tests. Is it the tests that need updating or does this break some other functionality?

Fixed the test failures in commit c69cb89. The issues were:

  1. case_when error in graph_labels.R - needed to extract vectors from data frame using [[]] instead of matrix subsetting
  2. Shape expectations - tests expected "circle" but now using numeric 21 (fillable circle shape)
  3. Scale functions - tests needed to use scale_fill_* instead of scale_color_* to match the new fill aesthetic

All 8 failing tests have been updated to work with the color-to-fill aesthetic changes.

@codecov

codecov Bot commented Feb 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.36364% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.71%. Comparing base (26b55d2) to head (c69cb89).

Files with missing lines Patch % Lines
R/graph_nodes.R 26.31% 14 Missing ⚠️
R/grapht.R 0.00% 5 Missing ⚠️
R/graph_labels.R 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   41.65%   41.71%   +0.05%     
==========================================
  Files          25       25              
  Lines        2019     2021       +2     
==========================================
+ Hits          841      843       +2     
  Misses       1178     1178              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jhollway

Copy link
Copy Markdown
Contributor

@copilot, can you improve code coverage by adding some more tests and/or using #nocov to exclude untestable aspects from the code coverage calculations?

Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>

Copilot AI commented Feb 19, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, can you improve code coverage by adding some more tests and/or using #nocov to exclude untestable aspects from the code coverage calculations?

Added in commit 6cbd678:

New tests (4 tests in test-graphr.R):

  • Verifies nodes use fill aesthetic instead of colour
  • Tests node_color with >2 values triggers scale_fill_manual with qualitative palette
  • Tests node_color with exactly 2 values triggers highlight palette
  • Tests two-mode networks get shape mappings

#nocov markers added to exclude untestable code:

  • R/grapht.R — entire file (requires gganimate/gifski for animation)
  • R/zzz.R.onAttach() (interactive startup messages)
  • R/autograph_utilities.RthisRequires()/thisRequiresBio() (interactive package install prompts)

@jhollway

Copy link
Copy Markdown
Contributor

The error is with the package RGraphViz, which cannot be found. It is used quite frequently in two-mode layout functions. Can you develop a fast and accurate replacement for a Sugiyama-type layout, @copilot? Does the code need to be in C++ or Rust to be performant?

Copilot AI and others added 2 commits February 24, 2026 08:51
Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>
Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>

Copilot AI commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

The error is with the package RGraphViz, which cannot be found. It is used quite frequently in two-mode layout functions. Can you develop a fast and accurate replacement for a Sugiyama-type layout, @c...

Replaced Rgraphviz with igraph::layout_with_sugiyama in commits a18a25d and 781d8bf. No C++/Rust needed — igraph already provides a performant Sugiyama implementation (C internally).

Changes:

  • layout_hierarchy(): Uses igraph::layout_with_sugiyama() with layers parameter for two-mode networks
  • layout_alluvial(): Same, with x/y swapped for left-to-right orientation
  • layout_lineage(): Removed thisRequiresBio("Rgraphviz") (calls alluvial internally)
  • Removed Rgraphviz from Enhances and BiocManager from Suggests in DESCRIPTION
  • Enabled hierarchy/two-mode tests on CI (removed skip_on_ci())
  • Added tests for default hierarchy and alluvial layouts

Copilot AI changed the title Change node aesthetics from 'color' to 'fill' (closes #12) Switch node aesthetics to 'fill' and replace Rgraphviz with igraph Sugiyama layout (closes #12) Feb 24, 2026
Copilot AI and others added 2 commits February 24, 2026 09:08
…mization

Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>
Co-authored-by: jhollway <5595229+jhollway@users.noreply.github.com>
Copilot AI changed the title Switch node aesthetics to 'fill' and replace Rgraphviz with igraph Sugiyama layout (closes #12) Switch node fill aesthetic and replace Rgraphviz with improved Sugiyama layout Feb 24, 2026
@jhollway jhollway closed this Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rely on fill instead of color for graphr/s/t functions for flexibility and consistency

2 participants