Skip to content

feat: detect parallel edges in directed and undirected graphs - #78

Open
huxiaoyi-ovo wants to merge 11 commits into
petercorke:mainfrom
huxiaoyi-ovo:feat/graph-hasparallel-75
Open

huxiaoyi-ovo wants to merge 11 commits into
petercorke:mainfrom
huxiaoyi-ovo:feat/graph-hasparallel-75

Conversation

@huxiaoyi-ovo

@huxiaoyi-ovo huxiaoyi-ovo commented Oct 15, 2026 •

Copy link
Copy Markdown

Summary

Implements parallel-edge detection and grouping for #75, including the improvements requested in the maintainer review.

  • Graph.isparallel() detects at least two distinct edges sharing endpoints in O(E) time, returning early on the first duplicate.
  • Graph.parallel_edges() returns list[tuple[Edge, ...]]. Groups are ordered by their first edge, and edges within each group preserve insertion order.
  • Both methods use a shared private endpoint-key helper.
  • Directed graphs distinguish opposite edge directions; undirected graphs do not. A single self-loop does not form a group.
  • Edge removal and reinsertion are reflected immediately without cached state.

Tests

Regression tests cover empty and simple graphs, three-edge groups, group ordering, edge removal and reinsertion, reversed edges, single and repeated self-loops, and directed/undirected graph construction.

An independent pairwise-reference test checks 500 seeded random graphs and verifies isparallel() == bool(parallel_edges()).

Documentation

Updated the API docstrings, README, docs/llms_intro.md, and CHANGELOG.

Verification

Fork CI: Passed

  • pytest: Python 3.9–3.14
  • mypy: Passed
  • Coverage job: Passed
  • Overall test gate: Passed

CI: https://github.com/huxiaoyi-ovo/pgraph-python/actions/runs/41025089369

Upstream GitHub Actions awaits maintainer approval.

AI disclosure: ChatGPT assisted with code analysis, implementation, and regression tests.

Fixes #75.

@codacy-production

codacy-production Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 35 complexity · 2 duplication

Metric Results
Complexity 35
Duplication 2

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@petercorke

Copy link
Copy Markdown
Owner

Thank you for your interest in pgraph-python, and for this contribution. isparallel() is clean, the O(E) approach with the early exit is what I hoped for in #75, and I like that the tests cover direction, self-loops and removal.

Before merging, I'd like to extend the PR in a few ways:

  1. Add Graph.parallel_edges(), returning list[tuple[Edge, ...]]: one tuple for each group of two or more edges that join the same vertices, in the order the edges were added, with the groups ordered by their first edge. It uses the same rules as isparallel() (direction matters in a DGraph, a single self-loop is not a group). I'd return a list, not a generator, because no group is known to be complete until the whole edge list has been scanned, and it matches edges().
  2. Share the key logic. Please move the (id(v1), id(v2)) computation, sorted for undirected graphs, into one private helper used by both methods, so they can't disagree about what is parallel. Keep the early exit in isparallel().
  3. Say "two or more" in the wording. The docstring and llms_intro.md talk about "two distinct edges" and a "parallel pair". That is right for isparallel(), which is true for any number of edges of at least two, but once groups are returned it could suggest that groups are always pairs. Something like "at least two distinct edges join the same vertices" would be clearer.

For tests, it would be great to extend your random-graph check so it asserts isparallel() == bool(parallel_edges()), and to add a few hand-written cases for parallel_edges() (a group of three, ordering, the undirected reverse pair, and a single and a double self-loop).

Also, please:

  • add a line to CHANGELOG.md,
  • document parallel_edges() in the README and docs/llms_intro.md next to isparallel(),
  • take the PR out of draft when it's ready, and I'll approve the CI run.

Thanks again, and let me know if you'd rather I made any of these changes myself.

@huxiaoyi-ovo

Copy link
Copy Markdown
Author

Hi Peter,

Thanks for the detailed review. I've addressed all your requests in the existing PR, with the latest changes in 7ba649b.

Graph.parallel_edges() now returns list[tuple[Edge, ...]], with complete groups ordered by their first edge and edges within each group in insertion order.

Both methods share the same private endpoint-key helper, while isparallel() retains its O(E) early exit. The directed/undirected distinction and self-loop semantics are preserved.

I've also updated the docstrings, README, docs/llms_intro.md, and CHANGELOG.

The expanded regression tests cover groups of three, insertion order, removal and reinsertion, reversed edges, self-loops, and 500 seeded random graphs checked against an independent pairwise reference.

The latest fork CI is fully green across Python 3.9–3.14, mypy, and coverage:

https://github.com/huxiaoyi-ovo/pgraph-python/actions/runs/41025089369

The PR is ready for review. Could you approve the upstream CI run when convenient?

Thanks again for your helpful feedback!

This branch has not been deployed

No deployments
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.

isparallel()

2 participants