Opened 7 years ago

Closed 7 years ago

#3160 closed defect (fixed)

[with patch, positive review] change is_planar for graphs to return bool

Reported by: was Owned by: rlm
Priority: major Milestone: sage-3.0.2
Component: graph theory Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

+1 on making this change. It's very unusual for an is_ function to
return anything but a bool :)
- Hide quoted text -

On Sun, May 11, 2008 at 11:34 AM, Robert Miller <rlmillster@gmail.com> wrote:
>
>>  On the other hand, that Jerin was confused maybe strongly suggests
>>  you might want to change the is_planar function to return True or
>>  False, and have another function or a flag to get the nonplanar
>>  subgroup.  In most of Sage foo.is_*() returns True or False, so maybe
>>  is_planar() is confusing, especially from a readability point of view.
>
> I think I agree. The default behavior should be True/False, and an
> option to return the present tuple should be available.
>
>

Attachments (2)

3160-bool-is-planar.patch (10.9 KB) - added by ekirkman 7 years ago.
3160-docs.patch (2.5 KB) - added by rlm 7 years ago.

Download all attachments as: .zip

Change History (5)

Changed 7 years ago by ekirkman

comment:1 Changed 7 years ago by ekirkman

  • Summary changed from change is_planar for graphs to return bool to [with patch, needs review] change is_planar for graphs to return bool

comment:2 Changed 7 years ago by rlm

  • Summary changed from [with patch, needs review] change is_planar for graphs to return bool to [with patch, positive review] change is_planar for graphs to return bool

-1 point for not testing before submitting!

Changed 7 years ago by rlm

comment:3 Changed 7 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged both patches in Sage 3.0.2.alpha1

Note: See TracTickets for help on using tickets.