#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) 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 17 months ago by jhpalmieri

  • Branch set to u/jhpalmieri/chain-complex-py3

comment:2 Changed 17 months ago by jhpalmieri

  • Commit set to 06fa117b8ea3d74093b7216a0535bfd4879a58ab
  • Status changed from new to needs_review

Regarding _latex_: there used to be an if...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:

06fa117trac 25940: Python 3 fixes for chain_complex.py

comment:3 Changed 17 months ago by tscrim

  • 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]
Last edited 17 months ago by tscrim (previous) (diff)

comment:4 Changed 17 months ago by git

  • Commit changed from 06fa117b8ea3d74093b7216a0535bfd4879a58ab to 0721159086bd74945039c402504ec992002384ed

Branch pushed to git repo; I updated commit sha1. New commits:

0721159trac 25940: use bool(deg < 0) instead of (deg < 0), just in case.

comment:5 Changed 17 months ago by jhpalmieri

Good idea.

comment:6 Changed 17 months ago by tscrim

  • Status changed from needs_review to positive_review

LGTM. Thanks.

comment:7 Changed 16 months ago by vbraun

  • 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.