Opened 3 years ago
Closed 3 years ago
#25505 closed defect (fixed)
Clean up __cinit__ methods of matrices
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage8.3 
Component:  linear algebra  Keywords:  
Cc:  tscrim  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  c8fa7bb (Commits, GitHub, GitLab)  Commit:  c8fa7bb27f4724a7263d76bec437ebcf7f87dec3 
Dependencies:  #25511  Stopgaps: 
Description (last modified by )
 Generic stuff like processing the
parent
should be done by the base class (matrix0.Matrix
in this case) instead of by each derived class individually. This means that we will require that the first argument of every matrix constructor is the parent. Because of #25511, this is currently true everywhere.
 In some cases,
__cinit__
calls__init__
of the base class. That is really backwards and should never be needed!
This is a requirement for #23719.
Change History (11)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
 Branch set to u/jdemeyer/clean_up___cinit___methods_of_matrices
comment:3 Changed 3 years ago by
 Cc tscrim added
 Commit set to 90a0b033a61df3b81793a160e6772ad5df7d051e
comment:4 Changed 3 years ago by
 Description modified (diff)
comment:5 Changed 3 years ago by
 Dependencies set to #25511
 Description modified (diff)
comment:6 Changed 3 years ago by
 Commit changed from 90a0b033a61df3b81793a160e6772ad5df7d051e to c8fa7bb27f4724a7263d76bec437ebcf7f87dec3
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4eae803  Making matrices use the new _echelon_in_place method.

e2f0550  Specify that _echelon_in_place shall return the pivots

3c4a06d  Enable _mul_long for matrices

1eaed37  More stuff in the meataxe interface, and a meataxe helper function

8a06c0f  Fix docstring formatting

13da208  Clean up creating Matrix_gfpn_dense matrices

cb52ee3  Mark one doctest optional

c8fa7bb  Clean up __cinit__ methods of matrices

comment:7 Changed 3 years ago by
 Status changed from new to needs_review
comment:8 Changed 3 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:9 followup: ↓ 10 Changed 3 years ago by
Does this also imply a positive review of the dependency #25511?
comment:10 in reply to: ↑ 9 Changed 3 years ago by
comment:11 Changed 3 years ago by
 Branch changed from u/jdemeyer/clean_up___cinit___methods_of_matrices to c8fa7bb27f4724a7263d76bec437ebcf7f87dec3
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
New commits:
Clean up __cinit__ methods of matrices