Opened 8 years ago
Closed 8 years ago
#18028 closed defect (fixed)
Remove GraphBundle
Reported by: | Volker Braun | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.6 |
Component: | graph theory | Keywords: | |
Cc: | Nathann Cohen, Jason Grout | Merged in: | |
Authors: | Volker Braun | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | bfb4443 (Commits, GitHub, GitLab) | Commit: | bfb444341d3b3a3f3a546338bc61f79085b9abf1 |
Dependencies: | Stopgaps: |
Description
Doesn't work and nothing changed over the last 5 years. If you want it back its always in the git repo.
Change History (9)
comment:1 Changed 8 years ago by
Branch: | → u/vbraun/remove_graphbundle |
---|
comment:2 Changed 8 years ago by
Authors: | → Volker Braun |
---|---|
Cc: | Nathann Cohen Jason Grout added |
Commit: | → bfb444341d3b3a3f3a546338bc61f79085b9abf1 |
Status: | new → needs_review |
comment:3 Changed 8 years ago by
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
comment:4 Changed 8 years ago by
Reviewers: | → Nathann Cohen |
---|---|
Status: | needs_review → positive_review |
comment:5 follow-up: 6 Changed 8 years ago by
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.
comment:6 Changed 8 years ago by
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
comment:7 Changed 8 years ago by
(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
comment:9 Changed 8 years ago by
Branch: | u/vbraun/remove_graphbundle → bfb444341d3b3a3f3a546338bc61f79085b9abf1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
New commits:
Delete GraphBundle