Opened 12 years ago

Closed 11 years ago

#10953 closed defect (fixed)

is_regular yields StopIterator error on empty graph

Reported by: Pål Grønås Drange Owned by: jason, ncohen, rlm
Priority: minor Milestone: sage-5.0
Component: graph theory Keywords: regular is_regular empty graph iterator
Cc: Michael Orlitzky Merged in: sage-5.0.beta1
Authors: Lukáš Lánský, Jeroen Demeyer Reviewers: Michael Orlitzky
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

The following statement yields an error:

sage: Graph().is_regular()

A quick fix to this is to test if the size of the graph is zero, in which case we return true. I assume the empty graph is k-regular for any k. If this is not the case for sage, another (better) error should be thrown.

Apply trac_10953_empty_graph_is_regular.4.patch

Attachments (4)

trac_10953_empty_graph_is_regular.patch (833 bytes) - added by Lukáš Lánský 11 years ago.
trac_10953_empty_graph_is_regular.2.patch (857 bytes) - added by Lukáš Lánský 11 years ago.
trac_10953_empty_graph_is_regular.3.patch (916 bytes) - added by Lukáš Lánský 11 years ago.
trac_10953_empty_graph_is_regular.4.patch (760 bytes) - added by Jeroen Demeyer 11 years ago.

Download all attachments as: .zip

Change History (18)

Changed 11 years ago by Lukáš Lánský

comment:1 Changed 11 years ago by Lukáš Lánský

Status: newneeds_review

comment:2 Changed 11 years ago by Michael Orlitzky

Cc: Michael Orlitzky added

You should include the ticket number in the doctest.

I would test self.order() rather than len(self) to save the reader from having to go look up what __len__ does, but that's just personal taste.

Changed 11 years ago by Lukáš Lánský

comment:3 Changed 11 years ago by Lukáš Lánský

Thanks! Is it better now?

comment:4 Changed 11 years ago by Michael Orlitzky

Almost! Would you mind moving the comment to its own line? There's nothing wrong with it at the moment, but in-line comments like that are given special treatment (e.g. #long, #optional, #todo...) so I think they should be avoided to prevent unexpected interactions.

Also: I find regularity defined "for all vertices..." so it is vacuously true that the empty graph is k-regular for any k.

I'm running a full ptestlong with the patch, and don't expect any problems.

comment:5 Changed 11 years ago by Lukáš Lánský

I agree that the empty graph is k-regular for any k -- I think the change works that way now. Or do you have in mind that I write another test for this?

comment:6 Changed 11 years ago by Michael Orlitzky

Nope, it's fine. I wasn't sure how regularity was defined, and your description said "I assume the empty graph is k-regular for any k", so I looked it up.

Changed 11 years ago by Lukáš Lánský

comment:7 Changed 11 years ago by Lukáš Lánský

Is this way of commenting possible?

Thanks for all the help. I'll try to not repeat those errors in my next patches. :-)

comment:8 Changed 11 years ago by Michael Orlitzky

Description: modified (diff)
Reviewers: Michael Orlitzky
Status: needs_reviewpositive_review

Perfect.

comment:9 Changed 11 years ago by Lukáš Lánský

Authors: Lukáš Lánský

comment:10 Changed 11 years ago by Jeroen Demeyer

Status: positive_reviewneeds_work

comment:11 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)
Milestone: sage-5.0
Status: needs_workneeds_review

Changed 11 years ago by Jeroen Demeyer

comment:12 Changed 11 years ago by Jeroen Demeyer

Authors: Lukáš LánskýLukáš Lánský, Jeroen Demeyer

Small fix to the doctest, needs review.

comment:13 Changed 11 years ago by Michael Orlitzky

Status: needs_reviewpositive_review

It passed another ptestlong, and the example looks good in the docs.

Sorry I missed the backticks, I finally fixed my dvipng and created a review checklist so I won't miss them in the future.

comment:14 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.0.beta1
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.