#22767 closed enhancement (fixed)
py3: some care for division
| Reported by: | chapoton | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | sage-8.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: |
Status badges
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_care-22767
- 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_care-22767
|
| 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_care-22767 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 filepy3 care for division in rigger configurationspy: care for division in an elliptic file