Remove GraphBundle

[vbraun]

Doesn't work and nothing changed over the last 5 years. If you want it back its always in the git repo.

CC: @nathanncohen @jasongrout

Component: graph theory

Author: Volker Braun

Branch/Commit: bfb4443

Reviewer: Nathann Cohen

Issue created by migration from https://trac.sagemath.org/ticket/18028

[vbraun]

Branch: u/vbraun/remove_graphbundle

[vbraun]

New commits:

bfb4443

Delete GraphBundle

[vbraun]

Author: Volker Braun

[vbraun]

Commit: bfb4443

[nathanncohen]

comment:3

Wow. Glad to see that it is actually possible to remove code from Sage. I always thought that it was 'politically impossible' to remove anything from this software, but fortunately I seem to be wrong.

What would you think of removing BipartiteGraph while we are at it ? This class' design is awful:

sage: g=BipartiteGraph()
sage: l=[(0,1),(3,2),(1,2),(0,3)]
sage: for e in l:
....:     g.add_edge(e)
RuntimeError: Edge vertices must lie in different partitions.

but

sage: g = Graph()
sage: g.add_edges(l)
sage: g.is_bipartite()
True

Also, I just noticed that one can very easily build a non-bipartite BipartiteGraph::

sage: g = BipartiteGraph()
sage: g.add_edges(graphs.CompleteGraph(30).edges())
sage: g.is_bipartite()
False

Please. Can you help me get rid of that class? I'll write the ticket if you agree.

Nathann

[nathanncohen]

Reviewer: Nathann Cohen

[vbraun]

comment:5

I've never used BipartiteGraph. Is it actually useful, just with a bad interface? Or completely non-functional? At least you seem to be able to construct one, maybe start with a deprecation and see if anybody complains.

[nathanncohen]

comment:6

I've never used BipartiteGraph. Is it actually useful, just with a bad interface? Or completely non-functional? At least you seem to be able to construct one, maybe start with a deprecation and see if anybody complains.

At some point it was claimed that it was "for teaching purposes", and it was badly designed from the start. add_edge does not check that the graph is bipartite, it checks that the previously computed bipartition stays valid, which is a wrong path to take for non-connected graphs.

Then you have the usual problems:

sage: g=BipartiteGraph(5)
sage: g.complement()
...
RuntimeError: Edge vertices must lie in different partitions.

Honestly if we must have such a class, we cannot afford to let it inherit from Graph. And from the way it appears in the doc, real graph users may want to give it a try, and regret it later.

Nathann

[nathanncohen]

comment:7

(basically its only 'feature' is to prevent you from making your graph non-bipartite. And given that it does that wrong, it is probably better if the users call .is_bipartite often instead of relying on the (wrong) exceptions raised by add_edge).

Nathann

[vbraun]

comment:8

If thats all then I'll be happy to review it...

[vbraun]

Changed branch from u/vbraun/remove_graphbundle to bfb4443