Opened 10 years ago
Closed 10 years ago
#10958 closed defect (fixed)
BipartiteGraph constructor without *args ignores **kwds
Reported by: | rhinton | Owned by: | rhinton |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | graph theory | Keywords: | bipartite graph |
Cc: | ncohen, dsm | Merged in: | sage-4.7.alpha4 |
Authors: | Ryan Hinton | Reviewers: | Nathann Cohen |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
For example,
sage: bg = BipartiteGraph(multiedges=True) sage: bg.allows_multiple_edges() False sage: bg = BipartiteGraph(Graph(), multiedges=True) sage: bg.allows_multiple_edges() True
Attachments (2)
Change History (14)
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 Changed 10 years ago by
- Status changed from needs_review to positive_review
Sounds fair :-)
Nathann
comment:3 Changed 10 years ago by
Shouldn't a working version of the example go into TESTS? I'd do it myself -- probably take less time than writing this message! :^)
-- but I'm not sure of the convention for when you should submit a patch to someone else's patch and don't want to step on toes.
comment:4 Changed 10 years ago by
Well... It could, but honestly on such short patches... I mean, this is something which should have been done when the original method was written, and it was forgotten. Adding 5 lines of doctest everytime we fix a mistake sounds a bit too much, even though more complicated bugs may ask for this "additional safeguard". :-)
"just my two cents"
Nathann
comment:5 Changed 10 years ago by
- Status changed from positive_review to needs_work
I think it's a good idea to add a test. I probably meant to, but forgot. :-) Updated patch coming shortly.
Changed 10 years ago by
comment:6 Changed 10 years ago by
- Reviewers set to ncohen
- Status changed from needs_work to needs_review
New patch posted including a doctest to verify correct behavior, passes tests for graph directory, running -testall now just for fun.
comment:7 Changed 10 years ago by
By the way, @dsm, I am always happy to have my toes stepped on if it saves time. :-)
comment:8 Changed 10 years ago by
- Cc dsm added
comment:9 Changed 10 years ago by
- Status changed from needs_review to positive_review
And in it goes, for the second time :-)
Nathann
comment:10 Changed 10 years ago by
- Reviewers changed from ncohen to Nathann Cohen
- Status changed from positive_review to needs_work
Change the commit message of the patch such that it contains the ticket number.
comment:11 Changed 10 years ago by
- Status changed from needs_work to positive_review
Apply only the second version of the patch.
Nathann
Changed 10 years ago by
comment:12 Changed 10 years ago by
- Merged in set to sage-4.7.alpha4
- Resolution set to fixed
- Status changed from positive_review to closed
One-line fix is attached.