Opened 19 months ago
Closed 19 months ago
#24548 closed enhancement (fixed)
py3: various details
Reported by: | chapoton | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | python3 | Keywords: | |
Cc: | tscrim, embray, jdemeyer | Merged in: | |
Authors: | Frédéric Chapoton | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 691e9de (Commits) | Commit: | 691e9de0f194e5928a41c750891b5ebd1d0d7007 |
Dependencies: | Stopgaps: |
Description (last modified by )
- deprecate some comparison-related methods in free modules
because the cmp keyword will go away in python3
- various changes about division
Change History (11)
comment:1 Changed 19 months ago by
- Description modified (diff)
- Summary changed from py3: deprecate some comparison-related methods in free modules to py3: various details
comment:2 Changed 19 months ago by
- Branch set to u/chapoton/24548
- Cc tscrim embray jdemeyer added
- Commit set to 691e9de0f194e5928a41c750891b5ebd1d0d7007
- Status changed from new to needs_review
comment:3 follow-up: ↓ 5 Changed 19 months ago by
I haven't studied the math in these cases but just to be sure, are you sure these are supposed to be integer division in all these cases? It looks like most of them want integer results, but I'd want to be sure. Have you tested these changes?
comment:4 Changed 19 months ago by
There could be some slight doubts only in
- src/sage/algebras/steenrod/steenrod_algebra_bases.py
- src/sage/modular/btquotients/pautomorphicform.py
but all tests pass in these files. And in these files, the cases of possible doubt are all wrapped with ZZ() or range().
So, this leaves me pretty confident that the math is ok.
comment:5 in reply to: ↑ 3 Changed 19 months ago by
Replying to embray:
I haven't studied the math in these cases but just to be sure, are you sure these are supposed to be integer division in all these cases? It looks like most of them want integer results, but I'd want to be sure. Have you tested these changes?
Also: adding from __future__ import division
affects all divisions in that file. In general, I think that adding from __future__ import division
is a good idea. But it does mean that one should verify all divisions.
comment:6 Changed 19 months ago by
green bot
comment:7 Changed 19 months ago by
ping ?
comment:8 Changed 19 months ago by
Apply failed?
comment:9 Changed 19 months ago by
but TestsPassed
on another patchbot..
comment:10 Changed 19 months ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
LGTM.
comment:11 Changed 19 months ago by
- Branch changed from u/chapoton/24548 to 691e9de0f194e5928a41c750891b5ebd1d0d7007
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
py#; various fixes, mostly about division