Opened 5 years ago

Closed 5 years ago

py3: future division in matrix2

Reported by: Owned by: chapoton major sage-8.1 python3 division Frédéric Chapoton André Apitzsch, Jeroen Demeyer N/A fdf0a68

Description

part of #15995

found using the "python -3" tests suggested there

comment:1 Changed 5 years ago by chapoton

Branch: → u/chapoton/23731 → e65a63e5cc7fda873d9ed34ded4d8c72d3a666f5 new → needs_review

New commits:

 ​e65a63e `py3: future division in matrix2`

comment:2 Changed 5 years ago by chapoton

Summary: py3: future division in matrix 2 → py3: future division in matrix2

comment:3 Changed 5 years ago by aapitzsch

Reviewers: → André Apitzsch needs_review → positive_review

LGTM.

comment:4 Changed 5 years ago by jdemeyer

Status: positive_review → needs_work

I disagree here: why hard-code `QQ`? Shouldn't we use `self._parent._base` instead?

```@@ -1371,7 +1372,7 @@ cdef class Matrix(Matrix1):
# now compute the permanental minor of the complement matrix if needed
if complement:
a = [one]
-            c1 = 1
+            c1 = QQ.one()
for k in range(1, mn + 1):
c1 = c1*(m-k+1)*(n-k+1)/k
c = c1
```

comment:5 Changed 5 years ago by chapoton

Because for this precise variable, we can stay in QQ. Otherwise, more complicated computations are done.

comment:6 Changed 5 years ago by git

Commit: e65a63e5cc7fda873d9ed34ded4d8c72d3a666f5 → fdf0a68c66108c7d370e5d23bff50d410e2f40a0

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

 ​fdf0a68 `trac 23731 try exact division`

comment:7 Changed 5 years ago by chapoton

So, what about the new commit ? The first makes sense (binomial coefficients) but i would not swear that the second one does make sense too.

New commits:

 ​fdf0a68 `trac 23731 try exact division`

comment:8 Changed 5 years ago by chapoton

Status: needs_work → needs_review

comment:9 Changed 5 years ago by jdemeyer

Reviewers: André Apitzsch → André Apitzsch, Jeroen Demeyer needs_review → positive_review

comment:10 Changed 5 years ago by vbraun

Branch: u/chapoton/23731 → fdf0a68c66108c7d370e5d23bff50d410e2f40a0 → fixed positive_review → closed