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:

Status badges

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)

trac_10958_bg_constructor_kwds.patch (1.2 KB) - added by rhinton 10 years ago.
trac_10958_bg_constructor_kwds.2.patch (1.2 KB) - added by ncohen 10 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 10 years ago by rhinton

  • Status changed from new to needs_review

One-line fix is attached.

comment:2 Changed 10 years ago by ncohen

  • Status changed from needs_review to positive_review

Sounds fair :-)

Nathann

comment:3 Changed 10 years ago by dsm

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 ncohen

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 rhinton

  • 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 rhinton

comment:6 Changed 10 years ago by rhinton

  • Authors set to rhinton
  • 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 rhinton

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 rhinton

  • Cc dsm added

comment:9 Changed 10 years ago by ncohen

  • Status changed from needs_review to positive_review

And in it goes, for the second time :-)

Nathann

comment:10 Changed 10 years ago by jdemeyer

  • Authors changed from rhinton to Ryan Hinton
  • 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 ncohen

  • Status changed from needs_work to positive_review

Apply only the second version of the patch.

Nathann

Changed 10 years ago by ncohen

comment:12 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.alpha4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.