Add graph_curve(::Graph)

[YueRen]

The final pull request of the Durham Workshop. Requires #4450, so I will rebase + undraft it once #4450 is merged.

[YueRen]

I've rebased the changes now that #4450 is merged.

@benlorenz: I also had to add an Base.keys(::EdgeIterator) to the combinatorics part to make edges(::Graph) work with findall, e.g.,

G = vertex_edge_graph(simplex(3))
findall(edge->(1 in edge),edges(G))

Let me know if there are any objections. (I normally try to have separate pull-requests for separate areas, but was too lazy to make this one-line change)

[benlorenz]

Adding keys would imply that eit[i] should work but that is not the case and would be difficult to make work. Also the length (and thus the new keys function) will change during iteration since the EdgeIterator is stateful.

julia> eit = edges(g)
Oscar.EdgeIterator(Polymake.LibPolymake.GraphEdgeIteratorAllocated{Undirected}(Ptr{Nothing} @0x000000003f407670), 6)

julia> length(eit)
6

julia> iterate(eit)
(Edge(2, 1), 2)

julia> iterate(eit)
(Edge(3, 1), 2)

julia> length(eit)
4

julia> collect(eit)
4-element Vector{Edge}:
 Edge(3, 2)
 Edge(4, 1)
 Edge(4, 2)
 Edge(4, 3)

[YueRen]

Adding keys would imply that eit[i] should work but that is not the case and would be difficult to make work. Also the length (and thus the new keys function) will change during iteration since the EdgeIterator is stateful.

Okay, I will remove Base.keys(::EdgeIterators) then and use collect(edges(G)) in findall.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.41%. Comparing base (7219bae) to head (50280d3).
Report is 97 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4452   +/-   ##
=======================================
  Coverage   84.41%   84.41%           
=======================================
  Files         668      669    +1     
  Lines       88441    88457   +16     
=======================================
+ Hits        74656    74672   +16     
  Misses      13785    13785           

Files with missing lines	Coverage Δ
src/AlgebraicGeometry/Curves/GraphCurve.jl	100.00% <100.00%> (ø)

[joschmitt]

This seems to be ready for review since a month. Maybe the triage meeting can discuss who can review it.

[fingolfin]

Perhaps @wdecker or @simonbrandhorst or @afkafkafk13 or @HechtiDerLachs or @ederc could review this?

[afkafkafk13]

Looks fine.

I left you one comment on the docstring. Feel free to ignore it, if the example is printed the way you want it.

[afkafkafk13]

The automatic suppression of the equations in printing makes the second example lack a bit of 'content'...

If you would like to have it for testing reasons, please put it in the tests instead and test for correctness of the curve. If you consider the information as vital for users, please print at least one of the equations.

[YueRen]

Good point, I will adjust the docstring accordingly.

[fingolfin]

Shouldn't this be like so? (Not sure if it ever matters... maybe for subgraphs...? Perhaps @lkastner can clarify?)

Suggested change

	for v in 1:n_vertices(G)
	for v in vertices(G)

[YueRen]

@fingolfin Because of the following: #4426

[lgoettgens]

Once #4633 is merged, this can (and should) be adapted

[fingolfin]

Thank you for clarifying