Skip to content

feat: make graph constructors default to undirected for consistency - #2453

Draft
krlmlr with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-graph-constructors-defaults
Draft

feat: make graph constructors default to undirected for consistency#2453
krlmlr with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-graph-constructors-defaults

Conversation

Copilot AI commented Oct 28, 2025

Copy link
Copy Markdown
Contributor

Graph constructors were inconsistent - some defaulted to directed (make_tree, make_star, make_graph) while most defaulted to undirected (sample_gnm, make_ring, make_full_graph). This creates unnecessary confusion.

Changes

  • make_tree(): Now defaults to mode = "undirected" (was "out")
  • make_star(): Now defaults to mode = "undirected" (was "in")
  • make_graph(): Now defaults to directed = FALSE (was TRUE)
  • Fixed bug where make_graph() ignored its default directed parameter when using numeric edge vectors
  • Updated deprecated aliases (graph(), graph.tree(), graph.star()) to match

Before/After

# Before (v1.x)
g <- make_tree(10)      # directed tree (mode = "out")
s <- make_star(5)       # directed star (mode = "in")  
m <- make_graph(1:10)   # directed graph

# After (v2.0)
g <- make_tree(10)      # undirected tree
s <- make_star(5)       # undirected star
m <- make_graph(1:10)   # undirected graph

# Directed graphs still available by explicit request
g <- make_tree(10, mode = "out")
s <- make_star(5, mode = "in")
m <- make_graph(1:10, directed = TRUE)

Breaking Change

This is a breaking change for v2.0. Code relying on implicit directed defaults must be updated to explicitly specify directed = TRUE or mode = "in"/"out".

Original prompt

This section details on the original issue you should resolve

<issue_title>Graph constructors are inconsistent in whether they create directed or undirected graphs</issue_title>
<issue_description>Some graph constructors create directed graphs by default, some create undirected ones, even when they can create either. This is confusing. Can we make the default undirected for all graph constructors in 2.0, when this makes sense? Opinions?

Examples

  • These are all undirected by default, even though they can create directed graphs as well: sample_gnm, make_ring, make_chordal_ring, make_full_graph, make_bipartite_graph (!) ...
  • These are all directed by default, even though they can create undirected graphs as well: make_tree, make_star, make_graph (!, compare make_bipartite_graph), ...
  • Of course, there are cases when there is only one reasonable choice for the directedness. make_kautz must be directed, make_from_prufer must be undirected, make_line_graph must follow the directness of its argument, make_directed_graph and make_undirected_graph must follow their naming, etc.

</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits October 28, 2025 22:03
Change make_tree(), make_star(), and make_graph() to default to undirected graphs for consistency with other graph constructors. Update tests accordingly.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fix issue where directed parameter wasn't being passed to old_graph when using default value, causing graphs to incorrectly default to directed. Update additional tests that relied on old defaults.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Fix remaining test failures by adding explicit directed=TRUE where tests expect directed graphs. Update snapshots for visual tests.

Co-authored-by: krlmlr <1741643+krlmlr@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix inconsistent graph constructors default behavior feat: make graph constructors default to undirected for consistency Oct 28, 2025
Copilot AI requested a review from krlmlr October 28, 2025 22:28
claude and others added 3 commits July 27, 2026 20:26
Carry this branch's undirected-by-default constructor change onto
main's migrated make functions:

- R/make.R: main's ellipsis signatures and ARG_HANDLE blocks with
  this branch's default order (make_star mode "undirected" first,
  make_tree likewise, make_graph()/graph() directed = FALSE);
  tools/migrations/make.R updated and blocks regenerated.
- Conflicted tests: main's igraph_local_seed()/lifecycle pins with
  this branch's explicit mode=/directed= call sites; main's
  impl-snapshot tests keep main's implicit calls (their snapshots
  now reflect the undirected default). Adapted main's
  alpha_centrality() coverage DAG to directed = TRUE.
- man pages re-rendered with roxygen2.

Branch-touched test files (make, aaa-auto, centrality, paths,
print-classic, structural-properties) pass FAIL 0 WARN 0. The full
suite still has failures in files this draft has not yet updated
for the new defaults (e.g. flow, conversion edge order) -- those
predate this merge and are the draft's remaining work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@github-actions

Copy link
Copy Markdown
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 2d0cce7 is merged into main:

  • ✔️as_adjacency_matrix: 804ms -> 809ms [-1.7%, +2.78%]
  • ✔️as_biadjacency_matrix: 778ms -> 778ms [-1.75%, +1.73%]
  • ✔️as_data_frame_both: 1.59ms -> 1.58ms [-2.56%, +1.22%]
  • ✔️as_long_data_frame: 3.98ms -> 4.15ms [-3.75%, +12.68%]
  • ✔️es_attr_filter: 2.76ms -> 2.79ms [-0.28%, +2.55%]
  • ✔️graph_from_adjacency_matrix: 139ms -> 140ms [-0.7%, +2.77%]
  • ✔️graph_from_data_frame: 3.59ms -> 3.59ms [-2.32%, +2.13%]
  • ✔️vs_attr_filter: 1.66ms -> 1.6ms [-11.04%, +4.23%]
  • ✔️vs_by_name: 1.02ms -> 1.05ms [-3.95%, +8.73%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

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.

Graph constructors are inconsistent in whether they create directed or undirected graphs

3 participants