Opened 4 years ago
Closed 4 years ago
#25940 closed defect (fixed)
Python 3: Chain complex fixes
Reported by: | jhpalmieri | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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 4 years ago by
- Branch set to u/jhpalmieri/chain-complex-py3
comment:2 Changed 4 years ago by
- Commit set to 06fa117b8ea3d74093b7216a0535bfd4879a58ab
- Status changed from new to needs_review
comment:3 Changed 4 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) <ipython-input-2-600718e2dfaa> in <module>() ----> 1 sorted([Integer(1),Integer(2),Integer(3),Integer(4)], reverse=(SR(-Integer(1)) < Integer(0))) /home/travis/sage-build/local/lib/python2.7/site-packages/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 4 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 4 years ago by
Good idea.
comment:7 Changed 4 years ago by
- Branch changed from u/jhpalmieri/chain-complex-py3 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