Opened 13 years ago

Closed 13 years ago

#6510 closed defect (fixed)

[with patch, positive review] Adds __nonzero__ method to abelian groups

Reported by: Taylor Sutton Owned by: tbd
Priority: minor Milestone: sage-4.1.1
Component: algebra Keywords: abelian groups
Cc: Merged in: sage-4.1.1.alpha0
Authors: Taylor Sutton Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

 sage: E=EllipticCurve([0,82])
 sage: tor=E.torsion_subgroup()
 sage: if tor:
 ...       print tor.order()
 1

We'd like to have tor evaluate to false in boolean context.

Attachments (3)

trac_6510.patch (677 bytes) - added by Taylor Sutton 13 years ago.
trac_6510.2.patch (677 bytes) - added by Taylor Sutton 13 years ago.
trac_6510.3.patch (936 bytes) - added by David Roe 13 years ago.

Download all attachments as: .zip

Change History (13)

Changed 13 years ago by Taylor Sutton

Attachment: trac_6510.patch added

comment:1 Changed 13 years ago by Taylor Sutton

Summary: Adds __nonzero__ method to abelian groups[with patch, needs review] Adds __nonzero__ method to abelian groups

comment:2 Changed 13 years ago by David Roe

Summary: [with patch, needs review] Adds __nonzero__ method to abelian groups[with patch, needs work] Adds __nonzero__ method to abelian groups

Needs a doctest.

Changed 13 years ago by Taylor Sutton

Attachment: trac_6510.2.patch added

comment:3 Changed 13 years ago by David Roe

Summary: [with patch, needs work] Adds __nonzero__ method to abelian groups[with patch, positive review] Adds __nonzero__ method to abelian groups

Looks good. All tests pass for me.

comment:4 Changed 13 years ago by ylchapuy

Summary: [with patch, positive review] Adds __nonzero__ method to abelian groups[with patch, needs work] Adds __nonzero__ method to abelian groups

Still needs a doctest!

comment:5 Changed 13 years ago by David Roe

Summary: [with patch, needs work] Adds __nonzero__ method to abelian groups[with patch, positive review] Adds __nonzero__ method to abelian groups

Now includes #indirect doctest

comment:6 Changed 13 years ago by Minh Van Nguyen

Reviewers: roedDavid Roe

What's the real name of tsutton? The real name should be used so we can give proper credit to contributors.

comment:7 Changed 13 years ago by Minh Van Nguyen

Summary: [with patch, positive review] Adds __nonzero__ method to abelian groups[with patch, needs work] Adds __nonzero__ method to abelian groups

I assume that I only need to apply the patch trac_6510.3.patch. But why are there three functions __nonzero__(self) all of which are the same and reside in the same module, but each block of definition contains different stuff? For example, after applying trac_6510.3.patch, I get the following in sage/groups/abelian_gps/abelian_group.py:

    def __nonzero__(self):
        return len(self.invariants()) != 0

    def __nonzero__(self):
        """                                                                     
        Returns True if this group is nontrivial.                               
                                                                                
        EXAMPLES::                                                              
                                                                                
            sage: E = EllipticCurve([0,82])                                     
            sage: T = E.torsion_subgroup()                                      
            sage: bool(T)                                                       
            False                                                               
        """
        return len(self.invariants()) != 0

    def __nonzero__(self):
        """                                                                     
        Returns True if this group is nontrivial.                               
                                                                                
        EXAMPLES::                                                              
                                                                                
            sage: E = EllipticCurve([0,82])                                     
            sage: T = E.torsion_subgroup()                                      
            sage: bool(T) # indirect doctest                                    
            False                                                               
        """
        return len(self.invariants()) != 0

Choose which block of function definition you want, and upload a new patch. Preferrably, you should replace trac_6510.3.patch with your new patch. I'm marking this as needs work. After a new patch is uploaded, positive review can be reinstated.

Changed 13 years ago by David Roe

Attachment: trac_6510.3.patch added

comment:8 Changed 13 years ago by David Roe

Summary: [with patch, needs work] Adds __nonzero__ method to abelian groups[with patch, positive review] Adds __nonzero__ method to abelian groups

Fixed. tsutton's real name is Taylor Sutton.

comment:9 Changed 13 years ago by Minh Van Nguyen

Authors: tsuttonTaylor Sutton

Merged the patch trac_6510.3.patch in sage-4.1.1-alpha0. I can't close this ticket because I don't have the privilege to do so. Sorry, folks :-(

comment:10 Changed 13 years ago by Minh Van Nguyen

Merged in: sage-4.1.1.alpha0
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.