Opened 2 years ago
Closed 2 years ago
#26716 closed enhancement (fixed)
py3: some small care for matrix
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.5 
Component:  python3  Keywords:  
Cc:  tscrim  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jeroen Demeyer, Vincent Klein 
Report Upstream:  N/A  Work issues:  
Branch:  574f478 (Commits, GitHub, GitLab)  Commit:  574f4789605096f8942ee818dd8a4e73b58b8fa3 
Dependencies:  Stopgaps: 
Description
Change History (14)
comment:1 Changed 2 years ago by
 Branch set to u/chapoton/26716
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Commit set to f8d780536b7fd34955a9b32743d0c4a287d5853d
comment:3 Changed 2 years ago by
 Commit changed from f8d780536b7fd34955a9b32743d0c4a287d5853d to 6d94e11dbaa3fecfadcd6a30d4e6e9046506c61d
comment:4 Changed 2 years ago by
 Owner changed from (none) to vklein
comment:5 Changed 2 years ago by
 Owner changed from vklein to (none)
comment:6 Changed 2 years ago by
 This change looks suspicious, can you justify it:
@@ 1927,6 +1928,7 @@ def block_matrix(*args, **kwds): sub_matrices = args[0] if is_Matrix(sub_matrices): + M = sub_matrices # a single matrix (check nrows/ncols/ring) if (nrows is not None and nrows != 1) or \ (ncols is not None and ncols != 1):
If it's a bug fix, then a doctest should be added.
 Replace
if ring == QQ or ring == ZZ:
byif ring is QQ or ring is ZZ:
 PEP 8 recommends to write
entries = lambda i, j: 1 / (i + j + 1)
as
def entries(i, j): return 1 / (i + j + 1)
comment:7 Changed 2 years ago by
 Commit changed from 6d94e11dbaa3fecfadcd6a30d4e6e9046506c61d to 574f4789605096f8942ee818dd8a4e73b58b8fa3
Branch pushed to git repo; I updated commit sha1. New commits:
574f478  trac 26716 adding a missing doctest, plus details

comment:8 Changed 2 years ago by
About 1. Pyflakes said that M was not defined anywhere. And indeed, this part of the code was not doctested.
 and 3. Done
comment:10 Changed 2 years ago by
 Owner changed from (none) to vklein
comment:11 Changed 2 years ago by
 Owner changed from vklein to (none)
Looks good to me.
Side Note : This ticket is actually a bug fix with the M = sub_matrices
modification.
comment:12 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:13 Changed 2 years ago by
 Reviewers set to Jeroen Demeyer, Vincent Klein
comment:14 Changed 2 years ago by
 Branch changed from u/chapoton/26716 to 574f4789605096f8942ee818dd8a4e73b58b8fa3
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
Branch pushed to git repo; I updated commit sha1. New commits:
py3: some care for matrix