[PartitionedGraphs] Small changes #114

jack-dunham · 2025-11-24T22:32:36Z

This PR includes the following changes to [PartitionedGraphs]

Added some additional NamedGraphs interface functions to various data types.
Constructing a graph from a QuotientView now directly calls quotient_graph.
Calling edges and vertices on QuotientView now returns according to the interface priority.
Constructing graphs from QuotientViews of AbstractSimpleGraphs will now return a graph of similar type.

- `vertices(::QuotientView)` now directly returns the keys `paritioned_vertices` return value. - `edges(::QuotientView)` now directly returns the edges of the `quotient_graph` return value (to coincide with interface overloading priority) - Converting a `QuotientView` to graph now calls `quotient_graph` directly - Adding methods for return directed/undirected graph types. Fix imports Fix rebase

…rtitionedGraph`

This behaves similarly to `partitionedgraph` function.

…ype by default This function can now be used `SimpleGraph` etc without promoting to `NamedGraph`.

This function constructs a graph with no edges, but with vertices of the quotient graph.

… depending on graph type.

…eating quotient graph

…ype as a parameter Allows for more generic quotient graph types.

…s for `PartitionedGraph`

…methods for `AbstractSimpleGraph`.

- argument `vertices` must be of type `Base.OneTo{Int}`.

…e, and triple argument methods. This interface is to overload `similar_graph(graph)`, i.e. the single argument method.

This function now acts similarly to `similar_graph`.

src/lib/GraphsExtensions/src/abstractgraph.jl

src/lib/GraphsExtensions/src/simplegraph.jl

src/lib/PartitionedGraphs/src/partitionedview.jl

test/test_namedgraph.jl

src/abstractnamedgraph.jl

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>

…nt vertices and edges.

mtfishman · 2025-12-18T17:50:47Z

@jack-dunham I had forgetten, but Base.to_index handles both scalar indexes and array of indices: https://github.com/JuliaLang/julia/blob/v1.12.3/base/indices.jl#L279-L315 (Base.to_indices refers to multiple dimensions). So maybe we could just go with to_graph_index for both single vertices/edges and sets of vertices/edges.

…bject when iterating.

…ions.

…ype constructor for `QuotientVertices`.

mtfishman · 2025-12-18T22:31:12Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+    end
+end
+
+quotients(qvs::QuotientVertexVertices) = getfield(qvs, :quotients)


I think this is a case where it would be better to not define this function and instead directly use qvs.quotients, to make it clearer this is just accessing data in the struct and not really a generic function.

mtfishman · 2025-12-19T22:04:44Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+    function QuotientEdges{V, E}(edges::Es) where {V, E, Es}
+        @assert E <: eltype(Es)
+        return new{V, E, Es}(edges)
+    end


Related to offline discussions, I think we should remove this constructor and just get the edge type from the input (we can revisit if use cases arise that don't have reasonable alternatives).

I thought I had checked this! Thanks for picking this up.

mtfishman · 2025-12-19T23:04:26Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+function Base.iterate(qes::QuotientEdges, state = nothing)
+    if isnothing(state)
+        out = iterate(qes.edges)
+    else
+        out = iterate(qes.edges, state)
+    end
+    if isnothing(out)
+        return nothing
+    else
+        (v, s) = out
+        return (QuotientEdge(v), s)
+    end
+end


The iterate definitions seem a bit complicated to me. It looks like they are all basically a lazy map over iterating over one of the fields. It seems like that can be implemented more simply like this:

struct Wrapper{T} data::T end Base.length(w::Wrapper) = length(w.data) Base.eltype(w::Wrapper) = String # Maybe use `Base.promote_op` if the map is more complicated? function Base.iterate(w::Wrapper, state...) return iterate(Iterators.map(string, w.data), state...) end

which enables:

julia> x = Wrapper([1, 2, 3]) Wrapper{Vector{Int64}}([1, 2, 3]) julia> collect(x) 3-element Vector{String}: "1" "2" "3"

I think then we wouldn't need iterate_graph_indices.

I don't like these iteration definitions either; I assumed there was a better solution. Thanks.

mtfishman · 2025-12-19T23:05:35Z

src/lib/PartitionedGraphs/src/quotientedge.jl

+    end
+end
+
+quotients(qes::QuotientEdgeEdges) = getfield(qes, :quotients)


Is this function needed? Could we just use direct field access?

mtfishman · 2025-12-19T23:11:37Z

src/abstractgraphindices.jl

+    Vertices(vertices::Vs) where {Vs} = new{eltype(Vs), Vs}(vertices)
+end
+
+Vertices(v1, v2, vertices...) = Vertices(vcat([v1, v2], collect(vertices)))


I'm not a fan of this constructor, I think we should keep constructors simpler and stricter, for example just have one kind of expected input (in this case, only accept a single collection of vertices). Wouldn't this kind of use case already be handled by the basic constructor, and you can pass a Tuple of vertices, i.e. Vertices(("a", "b", "c"))?

As a concrete issue with this constructor, it makes Vertices(["a"]) ambiguous, since it isn't clear if that is supposed to be the set of vertices, or just the length-1 case of passing multiple arguments (which then if I understand this constructor, would interpret ["a"] as a vertex itself). So it is better to just avoid that kind of ambiguity and keep constructors stricter.

That's fair. I mostly liked the syntax g[Vertices("a", "b")] when indexing as it cuts down on the number of nested brackets, i.e. g[Vertices(("a","b"))]. My motivation was that multiple arguments should be interpreted as a collection of vertices, and a single argument should be interpreted as an object representing a collection of vertices.

I realize it is probably not worth the inconsistency.

mtfishman · 2025-12-19T23:11:46Z

src/abstractgraphindices.jl

+    end
+end
+
+Edges(e1, e2, edges...) = Edges(vcat([e1, e2], collect(edges)))


Same comment as above.

mtfishman · 2025-12-19T23:12:51Z

src/abstractgraphindices.jl

+Base.iterate(gi::AbstractGraphIndices, state = nothing) = iterate_graph_indices(identity, gi, state)
+function iterate_graph_indices(f, gi::AbstractGraphIndices, state)
+    if isnothing(state)
+        out = iterate(parent_graph_indices(gi))
+    else
+        out = iterate(parent_graph_indices(gi), state)
+    end
+    if isnothing(out)
+        return nothing
+    else
+        (v, s) = out
+        return (f(v), s)
+    end
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:14:05Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+function Base.iterate(qvs::QuotientVertices, state = nothing)
+    return NamedGraphs.iterate_graph_indices(QuotientVertex, qvs, state)
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:14:31Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+function Base.iterate(qvs::QuotientVertexVertices, state = nothing)
+    return NamedGraphs.iterate_graph_indices(v -> quotient_index(qvs)[v], qvs, state)
+end


Same comment as above regarding a simpler iterate definition.

mtfishman · 2025-12-19T23:16:41Z

src/lib/PartitionedGraphs/src/quotientvertex.jl

+    end
+end
+
+QuotientVertices(vertices...) = QuotientVertices(collect(vertices))


Same comment as the ones for the analogous Vertices and Edges constructors.

jack-dunham added 24 commits November 24, 2025 16:44

[PartitionedGraphs] Add directed/undirected graph type methos for `Pa…

be7d219

…rtitionedGraph`

[PartitionedGraphs] Add similar_type method for QuotientView.

e453d7f

[PartitionedGraphs] Add quotientview interface function

9487b51

This behaves similarly to `partitionedgraph` function.

[PartitionedGraphs] quotient_graph now returns a graph of similar t…

6bfb9b4

…ype by default This function can now be used `SimpleGraph` etc without promoting to `NamedGraph`.

Version bump

a7d5a58

[PartitionGraphs] fallback method for show

b6f1f6b

Fix missing import

3702ef6

Remove broken import

4dc4611

[PartitionedGraphs] Add edgeless_quotient_graph interface function.

f1641a2

This function constructs a graph with no edges, but with vertices of the quotient graph.

[PartitionedGraphs] quotient_graph now returns a (un)directed graph…

36381cf

… depending on graph type.

[PartitionedGraphs] Explictly construct edges of correct type when cr…

703363a

…eating quotient graph

[PartitionedGraphs] PartitionedGraph now takes the quotient graph t…

d277466

…ype as a parameter Allows for more generic quotient graph types.

Fix typo as graph_type -> graph.

e55cb9f

[PartitionedGraphs] Add directed/undireced graph constructor function…

fa2608c

…s for `PartitionedGraph`

Remove redundant directed_graph method, add fallbacks NamedGraph …

e81da68

…methods for `AbstractSimpleGraph`.

[PartitionedGraphs] Remove overlapping method

d7fe1ee

Add fall back for position_graph_type based on promote_op.

809f5b8

Test structs now implemented the correct interface for NamedGraphs

02789eb

Add add_vertices! method for AbstractSimpleGraph;

0ae1425

- argument `vertices` must be of type `Base.OneTo{Int}`.

Rename graph_from_vertices -> similar_graph and add single, doubl…

e1627a9

…e, and triple argument methods. This interface is to overload `similar_graph(graph)`, i.e. the single argument method.

Rename edgeless_quotient_graph to similar_quotient_graph.

1929506

This function now acts similarly to `similar_graph`.

Add tests for similar_graph function

3cd86a3

Add specific and fallback methods for Base.reverse with tests.

4ae23ee

mtfishman reviewed Dec 8, 2025

View reviewed changes

src/lib/GraphsExtensions/src/abstractgraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/lib/GraphsExtensions/src/simplegraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/lib/PartitionedGraphs/src/partitionedview.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

test/test_namedgraph.jl Outdated Show resolved Hide resolved

mtfishman reviewed Dec 9, 2025

View reviewed changes

src/abstractnamedgraph.jl Show resolved Hide resolved

Remove left over @warn statement in test file.

d6d98fa

Co-authored-by: Matt Fishman <mtfishman@users.noreply.github.com>

jack-dunham added 6 commits December 15, 2025 18:01

No longer collect vertices/edges when converting quotients

3a7591c

Basic graph indexing tests for partitioned graphs

25f524e

Fix incorrect name in error messages

f8f38e4

Add QuotientVertices/Edges structs as wrappers around lists of quotie…

caa1772

…nt vertices and edges.

Deprec AbstractGraphIndices indexing

c5bcfe5

Overhaul edge/vertex collection handling.

4e15989

jack-dunham added 5 commits December 18, 2025 13:19

QuotientEdges/Vertices now returns a QuotientEdge or QuotientVertex o…

9ae8f4f

…bject when iterating.

Use to_graph_index instead of to_graph_indexing and related funct…

5b977c9

…ions.

Fix convert_vertextype methods for AbstractNamedGraph.

f569a9a

Remove Sub from QuotientXSubX struct names; remove explict vertex t…

20c385c

…ype constructor for `QuotientVertices`.

Cosmetic changes

a4a1ad0

mtfishman reviewed Dec 18, 2025

View reviewed changes

jack-dunham added 2 commits December 19, 2025 16:36

Expand on vertex indexing.

2138b82

Allow subgraphing w.r.t QuotientVertexVertex

453f352

mtfishman reviewed Dec 19, 2025

View reviewed changes

jack-dunham added 6 commits January 5, 2026 10:09

Remove redundant field getter quotients.

196561e

Remove abiguous vararg constructor for QuotientVertices.

622f1d8

Remove ambiguous vararg constructors from Vertices and Edges.

93f3ead

Simplify iterate implementation for AbstractGraphIndex subtypes.

6392a3e

Improve consistency between Edges/Vertices code + reorganisation

028e846

Add subgraphing tests

92579ad

[PartitionedGraphs] Small changes #114

Are you sure you want to change the base?

[PartitionedGraphs] Small changes #114

Uh oh!

Conversation

jack-dunham commented Nov 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtfishman commented Dec 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtfishman Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mtfishman Dec 19, 2025 •

edited

Loading