Opened 3 years ago
Closed 3 years ago
#25940 closed defect (fixed)
Python 3: Chain complex fixes
Reported by:  jhpalmieri  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  python3  Keywords:  sagedays@icerm 
Cc:  chapoton  Merged in:  
Authors:  John Palmieri  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  0721159 (Commits, GitHub, GitLab)  Commit:  0721159086bd74945039c402504ec992002384ed 
Dependencies:  Stopgaps: 
Description
This implements a hash function for chain complexes, and it also fixes a sorting issue in the _latex_
method. Combined with #25935, this makes tests pass for src/sage/homology/chain_*
with Python 3.
Change History (7)
comment:1 Changed 3 years ago by
 Branch set to u/jhpalmieri/chaincomplexpy3
comment:2 Changed 3 years ago by
 Commit set to 06fa117b8ea3d74093b7216a0535bfd4879a58ab
 Status changed from new to needs_review
comment:3 Changed 3 years ago by
 Keywords sagedays@icerm added
 Reviewers set to Travis Scrimshaw
LGTM modulo one probably extremely unlikely corner case if deg
is in SR
:
sage: SR(1) < 0 1 < 0 sage: sorted([1,2,3,4], reverse=(SR(1) < 0))  ValueError Traceback (most recent call last) <ipythoninput2600718e2dfaa> in <module>() > 1 sorted([Integer(1),Integer(2),Integer(3),Integer(4)], reverse=(SR(Integer(1)) < Integer(0))) /home/travis/sagebuild/local/lib/python2.7/sitepackages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.__int__ (build/cythonized/sage/symbolic/expression.cpp:9460)() 1112 rif_self = sage.rings.all.RIF(self) 1113 except TypeError: > 1114 raise ValueError("cannot convert %s to int" % self) 1115 if rif_self > 0 or (rif_self.contains_zero() and self > 0): 1116 result = floor(self) ValueError: cannot convert 1 < 0 to int
The fix would be bool(deg < 0)
:
sage: sorted([1,2,3,4], reverse=bool(SR(1) < 0)) [4, 3, 2, 1]
comment:4 Changed 3 years ago by
 Commit changed from 06fa117b8ea3d74093b7216a0535bfd4879a58ab to 0721159086bd74945039c402504ec992002384ed
Branch pushed to git repo; I updated commit sha1. New commits:
0721159  trac 25940: use bool(deg < 0) instead of (deg < 0), just in case.

comment:5 Changed 3 years ago by
Good idea.
comment:7 Changed 3 years ago by
 Branch changed from u/jhpalmieri/chaincomplexpy3 to 0721159086bd74945039c402504ec992002384ed
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
Regarding
_latex_
: there used to be anif...else
block, depending on whether the chain complex was graded over Z or over some other group. First, there was a bug in the code, so it was printing the wrong thing, and also printing incomplete information, in the non Z case. Second, the special case is not needed: the Z code works just as well for other cases, I think. There may be problems if the grading group is not free abelian, I'm not sure. In any case, this is an improvement over the previous situation.New commits:
trac 25940: Python 3 fixes for chain_complex.py