Opened 9 years ago
Closed 8 years ago
#10953 closed defect (fixed)
is_regular yields StopIterator error on empty graph
Reported by: | pgdx | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.0 |
Component: | graph theory | Keywords: | regular is_regular empty graph iterator |
Cc: | mjo | 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: |
Description (last modified by )
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.
Attachments (4)
Change History (18)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
comment:2 Changed 8 years ago by
- Cc mjo added
Changed 8 years ago by
comment:3 Changed 8 years ago by
Thanks! Is it better now?
comment:4 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
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 8 years ago by
comment:7 Changed 8 years ago by
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 8 years ago by
- Description modified (diff)
- Reviewers set to Michael Orlitzky
- Status changed from needs_review to positive_review
Perfect.
comment:9 Changed 8 years ago by
comment:10 Changed 8 years ago by
- Status changed from positive_review to needs_work
comment:11 Changed 8 years ago by
- Description modified (diff)
- Milestone set to sage-5.0
- Status changed from needs_work to needs_review
Changed 8 years ago by
comment:13 Changed 8 years ago by
- Status changed from needs_review to positive_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 8 years ago by
- Merged in set to sage-5.0.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
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.