#22767 closed enhancement (fixed)
py3: some care for division
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  python3  Keywords:  division 
Cc:  tscrim, jmantysalo, aapitzsch, jdemeyer  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  9e38f0e (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
correct some of the remaining failures when division uses python3 behaviour
part of #15995
Change History (17)
comment:1 Changed 5 years ago by
 Branch set to u/chapoton/22767
 Commit set to cce963481dbc72cae81fe9c21d1c1be827d16c67
 Status changed from new to needs_review
comment:2 Changed 5 years ago by
 Status changed from needs_review to needs_info
Why change int
to Integer
in various places? I don't think that is the right way to fix things.
comment:3 Changed 5 years ago by
 Cc tscrim jmantysalo aapitzsch jdemeyer added
To which of the 3 changed file do you refer ?
I must admit that I have made so that it works, but not tried to understand why.
comment:4 Changed 5 years ago by
@tscrim: "src/sage/combinat/crystals/virtual_crystal.py" also needs care:
Try adding from __future__ import division
there, you will get 4 failures.
comment:5 Changed 5 years ago by
 Branch changed from u/chapoton/22767 to public/python3/division_care22767
 Commit changed from cce963481dbc72cae81fe9c21d1c1be827d16c67 to 02c2b88accabeb4887916aae42d039e90b93c83e
 Reviewers set to Travis Scrimshaw
I know the rigged configuration code, so I made the necessary change there. I also fixed the issues in virtual_crystals.py
. I have no idea for the other two.
New commits:
69c0491  Make virtual_crystal.py python3 compatible.

f945267  Merge branch 'u/chapoton/22767' of git://trac.sagemath.org/sage into public/python3/division_care22767

02c2b88  Change for cocharge of rigged configuration elements.

comment:6 Changed 5 years ago by
may we also undo this change in rigged configurations ?
 cc = 0  rigging_sum = 0 + cc = ZZ.zero() + rigging_sum = ZZ.zero()
comment:7 Changed 5 years ago by
These are good to have as cc
is eventually exposed to the user, and I think could remain an int
in certain situations.
comment:8 Changed 5 years ago by
 Status changed from needs_info to needs_review
ok, then setting back to needs review
comment:9 Changed 5 years ago by
 Status changed from needs_review to needs_work
need to do something in the elliptic file
comment:10 Changed 5 years ago by
 Commit changed from 02c2b88accabeb4887916aae42d039e90b93c83e to 9e38f0e49e9799b024c114cceafd23c1b252db01
comment:11 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:12 Changed 5 years ago by
Perhaps this is bikeshedding, but I don't understand why this change
 r /= v.ramification_index() * v.residue_class_degree() + r = r / (v.ramification_index() * v.residue_class_degree())
You don't need to change it back, but it just feels like it is slightly more complicated code to me.
comment:13 Changed 5 years ago by
Well, yes, you are right. I was in doubt to what kind of division is performed by /=
.
comment:14 Changed 5 years ago by
If the patchbot comes back green, then you can set a positive review.
comment:15 Changed 5 years ago by
 Status changed from needs_review to positive_review
ok,one bot was green
comment:16 Changed 5 years ago by
 Branch changed from public/python3/division_care22767 to 9e38f0e49e9799b024c114cceafd23c1b252db01
 Resolution set to fixed
 Status changed from positive_review to closed
comment:17 Changed 4 years ago by
 Commit 9e38f0e49e9799b024c114cceafd23c1b252db01 deleted
 Keywords division added
New commits:
py3 add from future import division in one file
py3 care for division in rigger configurations
py: care for division in an elliptic file