Opened 3 years ago
Closed 3 years ago
#28198 closed enhancement (fixed)
Add method is_bipartite to BipartiteGraph
Reported by:  dcoudert  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  graph theory  Keywords:  
Cc:  Merged in:  
Authors:  David Coudert  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  0436491 (Commits, GitHub, GitLab)  Commit:  0436491078f2d5ee834bfdfcd07a996f539b5754 
Dependencies:  Stopgaps: 
Description
A member of class BipartiteGraph
is obviously bipartite, so we can avoid to test it.
Change History (18)
comment:1 Changed 3 years ago by
 Branch set to public/graphs/28198_is_bipartite
 Commit set to d239b5ef03368c540b5ee1227a047dbf7d148eda
 Status changed from new to needs_review
comment:2 Changed 3 years ago by
Insteed of having a weird and mostly useless special case
+ if not hasattr(self, 'left') or not hasattr(self, 'right'): + # This case may occur during the initialization (i.e., in __init__) + return GenericGraph.is_bipartite(self, certificate=certificate)
Why don't you call GenericGraph.is_bipartite(self)
in __init__
instead of `self.is_bipartite()?
comment:3 Changed 3 years ago by
 Commit changed from d239b5ef03368c540b5ee1227a047dbf7d148eda to 564fa99c10956c63139ad0dbd9075dd1f2c020c4
Branch pushed to git repo; I updated commit sha1. New commits:
564fa99  proposition of simplification for 28198

comment:4 Changed 3 years ago by
 Commit changed from 564fa99c10956c63139ad0dbd9075dd1f2c020c4 to c909cdcbbf0601b99351b3abe5a01dd38f1682a9
Branch pushed to git repo; I updated commit sha1. New commits:
c909cdc  trac #28198: small correction

comment:5 Changed 3 years ago by
I agree with your improvement. I did a small correction to avoid this issue
sage: a = b = set() sage: a.add(1) sage: b {1}
For me this ticket is good to go. I let you conclude. Thank you.
comment:6 Changed 3 years ago by
 I don't like so much the duplication in the
BipartiteGraph.__init__
.  The
sets_from_colors
would be more natural if it would returnans
instead ofans.values()
. That is it would transform a dictionaryk > v
inv > {keys with that values}
.
Is that ok if I try something to fix these.
comment:7 Changed 3 years ago by
Sure, the branch is in public so that we can interact (and I'm not the author of the commit "proposition of simplification for 28198")
comment:8 Changed 3 years ago by
 Commit changed from c909cdcbbf0601b99351b3abe5a01dd38f1682a9 to f2e3d83a91a55c61b3a138b0271a258d3e6211b5
Branch pushed to git repo; I updated commit sha1. New commits:
f2e3d83  cleanup

comment:9 Changed 3 years ago by
I like it better now :)
comment:10 Changed 3 years ago by
 Reviewers set to Vincent Delecroix
comment:11 Changed 3 years ago by
 Commit changed from f2e3d83a91a55c61b3a138b0271a258d3e6211b5 to 17d54019a0aedc0744999ebb7547b867d4e9246b
Branch pushed to git repo; I updated commit sha1. New commits:
17d5401  even simpler

comment:12 Changed 3 years ago by
we can simplify more putting the content of sets_from_colors
inside _upgrade_from_graph
, no ?
comment:13 Changed 3 years ago by
make sense
comment:14 Changed 3 years ago by
 Commit changed from 17d54019a0aedc0744999ebb7547b867d4e9246b to 0436491078f2d5ee834bfdfcd07a996f539b5754
Branch pushed to git repo; I updated commit sha1. New commits:
0436491  trac #28198: more simplifications

comment:15 Changed 3 years ago by
Should be good now.
comment:16 Changed 3 years ago by
 Status changed from needs_review to positive_review
good for me at least
comment:17 Changed 3 years ago by
Thank you.
comment:18 Changed 3 years ago by
 Branch changed from public/graphs/28198_is_bipartite to 0436491078f2d5ee834bfdfcd07a996f539b5754
 Resolution set to fixed
 Status changed from positive_review to closed
If you have in mind a smarter way to handle initialization, please share it. The branch is in public, so you can implement it directly.
New commits:
trac #28198: is_bipartite for BipartiteGraph