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

The name/ family name of the (di)graph is preserved on addition/ deletion of a vertex/ an edge. #38768

Open
2 tasks done
janmenjayap opened this issue Oct 4, 2024 · 7 comments

Comments

@janmenjayap
Copy link
Contributor

Steps To Reproduce

Consider the following example:
Upon deleting a vertex from the Petersen graph, the string representation of the resulting graph still states that the new graph is the Petersen graph.

sage: P = graphs.PetersenGraph()
sage: P
Petersen graph: Graph on 10 vertices
sage: P.delete_vertex(0)
sage: P
Petersen graph: Graph on 9 vertices

Upon deleting an edge from a cycle graph, the string representation of the resulting graph still states that the new graph is a cycle graph:

sage: G = graphs.CycleGraph(4)
sage: G
Cycle graph: Graph on 4 vertices
sage: G.delete_edge(0, 1)
sage: G
Cycle graph: Graph on 4 vertices

An analogous result can be observed for each instance of a named (di)graph/ (di)graph of a particular family. An example for digraph goes as follows:

sage: D = digraphs.Circuit(5)
sage: D
Circuit: Digraph on 5 vertices
sage: D.delete_edge(0, 1)
sage: D
Circuit: Digraph on 5 vertices

Analogous behaviour may be observed in case of addition of a vertex/ an edge:

sage: G = graphs.CycleGraph(4)
sage: G
Cycle graph: Graph on 4 vertices
sage: G.add_vertex()
4
sage: G
Cycle graph: Graph on 5 vertices

Expected Behavior

A (possible) expected behaviour can be to remove the prefix substring pertaining to the family name/ name of the (di)graph on the addition/ deletion of a vertex/ an edge. The expected behaviour for the above mentioned examples are listed below:

sage: P = graphs.PetersenGraph()
sage: P
Petersen graph: Graph on 10 vertices
sage: P.delete_vertex(0)
sage: P
Graph on 9 vertices
sage: G = graphs.CycleGraph(4)
sage: G
Cycle graph: Graph on 4 vertices
sage: G.delete_edge(0, 1)
sage: G
Graph on 4 vertices
sage: D = digraphs.Circuit(5)
sage: D
Circuit: Digraph on 5 vertices
sage: D.delete_edge(0, 1)
sage: D
Digraph on 5 vertices
sage: G = graphs.CycleGraph(4)
sage: G
Cycle graph: Graph on 4 vertices
sage: G.add_vertex()
4
sage: G
Graph on 5 vertices

Actual Behavior

The name/ family name of the (di)graph is preserved on addition/ deletion of a vertex/ an edge. For examples see the section: Steps to reproduce

Additional Information

This problem may be solved by setting self.name() or the output of self.str() to '' on the addition/ deletion of a vertex/ an edge.

This issue shall also be corrected for the addition of cliques, cycles, paths and removable of multiedges.

Environment

  • OS: Ubuntu 20.04.6 LTS (Focal Fossa)
  • Sage Version: 10.5.beta6

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@dcoudert
Copy link
Contributor

dcoudert commented Oct 8, 2024

I'm not in favor of such change. Indeed, it would require extra work in add/delete vertex/edge methods, and we try to make these critical methods as efficient as possible. I know it's only one assignment, but it must be done only if the graph is effectively modified. For instance, G.add_vertex(0) does not modify the graph is vertex 0 is already in the graph.

@mantepse
Copy link
Collaborator

mantepse commented Oct 8, 2024

How about giving names only to immutable graphs?

@dcoudert
Copy link
Contributor

dcoudert commented Oct 8, 2024

Most of our graphs generators graphs.<TAB> return mutable graphs, and these graphs are names (e.g., Petersen graph). I don't think it is a good idea to force the generators to return immutable graphs only or to add a parameter like immutable=True to each generator.

@mantepse
Copy link
Collaborator

mantepse commented Oct 8, 2024

I actually do think that it would be good to return immutable graphs by default.

@dcoudert
Copy link
Contributor

dcoudert commented Oct 8, 2024

This can certainly be done, but it's a significant amount of work since we have hundreds of generators...

@janmenjayap
Copy link
Contributor Author

I would like to suggest the following approach:

  1. While deleting vertices/ edges, if all the vertices/ edges are existing vertices/ edges, we can just simply change self.name() to ''.
  2. Similarly, if we are adding at least one new vertex/ at least one new edge, we can just simply change self.name() to ''.
  3. Analogous alteration can be made for addition of a path/ a cycle/ a clique.

For the places where these methods are used, we could do a global search for these methods and implement the necessary updates. While I understand this might require additional effort, I believe it will be worthwhile in the long run :)

@dcoudert
Copy link
Contributor

dcoudert commented Oct 9, 2024

As I said in #38768 (comment), I'm not in favor of such change that may slowdown many methods for a very small benefit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants