Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different code- vs. label- API for has_edge vs. add_edge! #78

Open
thchr opened this issue Mar 25, 2024 · 6 comments
Open

Different code- vs. label- API for has_edge vs. add_edge! #78

thchr opened this issue Mar 25, 2024 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@thchr
Copy link
Contributor

thchr commented Mar 25, 2024

Example below:

g = SimpleGraph()
mg = MetaGraph(g, String, Nothing, Nothing)
for c in 'a':'d'
    add_vertex!(mg, string(c))
end
add_edge!(mg, "a", "b")
has_edge(mg, "a", "b")
ERROR: method has_edge not implemented.

I.e., add_edge! works on the labels of the MetaGraph but has_edge works with the codes. This is especially surprising if you (contrary to recommendations) go with Integer labels:

g = SimpleGraph()
mg = MetaGraph(g, Int, Nothing, Nothing) # <--- Int label
for i in 3:6
    add_vertex!(mg, i)
end
add_edge!(mg, 3, 4)
has_edge(mg, 3, 4)

(where, current API requires us to do has_edge(mg, code_for(mg, 3), code_for(mg, 4)) to get a meaningful result.

@gdalle
Copy link
Member

gdalle commented Mar 25, 2024

You're right, a function has_edge(mg, label1, label2) would be warranted.
But beware that since has_edge(mg, code1::Integer, code2::Integer) has to exist, if you're using integers as labels, not sure which of the two methods would get picked.

@thchr
Copy link
Contributor Author

thchr commented Mar 25, 2024

Yeah, that's a potential issue indeed: but should there really be a code-access has_edge for a MetaGraph? It seems much more natural to require that one does has_edge(mg.graph, code1, code2) if one wants to query the underlying graph? Or, is this to comply with a Graphs.jl interface?

Alternatively, has there been any discussion of introducing a special Label or Code type? In the latter case, it could simply wrap an integer.

@gdalle
Copy link
Member

gdalle commented Mar 25, 2024

It is necessary to comply with the Graphs.jl interface, otherwise you'd have to call all graph algorithms with algorithm(mg.graph, code)

@thchr
Copy link
Contributor Author

thchr commented Mar 26, 2024

I see. That's too bad.

Is there a sense in which some of these issues could be alleviated if Graphs.jl had an internal type like

struct Vertex{T<:Integer}
   v :: T
end

with the iterates of vertices(g) then being of type Vertex? This would be symmetric with Edge{T} and edges(g) and seems it would fix many of these issues. I suppose it might be breaking - but is it part of the consideration for Graphs 2.0?

@gdalle
Copy link
Member

gdalle commented Mar 26, 2024

One of the key drivers behind Graphs.jl 2.0 conversations is the desire to support arbitrary vertex types. In that sense, we will no longer need the distinction between labels and vertices, at least on a surface level

@gdalle
Copy link
Member

gdalle commented Mar 27, 2024

As of right now I don't think there is a good solution to your problem, aside from documenting this caveat. Do you want to open a PR?
(I haven't forgotten the simple paths one, no worries)

@gdalle gdalle added the documentation Improvements or additions to documentation label Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants