Opened 4 years ago
Closed 4 years ago
#23068 closed enhancement (fixed)
py3 more absolute imports in pyx files
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  python3  Keywords:  
Cc:  jdemeyer  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jeroen Demeyer, André Apitzsch 
Report Upstream:  N/A  Work issues:  
Branch:  7113b26 (Commits, GitHub, GitLab)  Commit:  7113b26fdcc440dbf4c2d6c94c0a08d40d963e42 
Dependencies:  Stopgaps: 
Description
part of #22808
Change History (11)
comment:1 Changed 4 years ago by
 Branch set to u/chapoton/23068
 Commit set to 030b0b596d1e146858c78c3eb9b6b138823911ae
comment:2 Changed 4 years ago by
 Commit changed from 030b0b596d1e146858c78c3eb9b6b138823911ae to b7eedc59e8eebaa135b27dd8ed9a78b9ccc24d9e
Branch pushed to git repo; I updated commit sha1. New commits:
b7eedc5  trac 23068 typo

comment:3 Changed 4 years ago by
 Commit changed from b7eedc59e8eebaa135b27dd8ed9a78b9ccc24d9e to a12d70010393a07196b4b451be1ccc153d33e1b2
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a12d700  more absolute imports in pyx files

comment:4 Changed 4 years ago by
 Commit changed from a12d70010393a07196b4b451be1ccc153d33e1b2 to 59237752defce7bffd861e80591da3246ec32d09
Branch pushed to git repo; I updated commit sha1. New commits:
5923775  trac 23068 fixing imports in matrix2

comment:5 Changed 4 years ago by
 Cc jdemeyer added
 Status changed from new to needs_review
bot is essentially green, please review
comment:6 Changed 4 years ago by
 Status changed from needs_review to needs_work
 Why did you change
if self._ncols == 0
to
if not self._ncols
I find self._ncols == 0
clearer to understand: it checks for a matrix with zero colomns.
 It is wrong to write
sage.matrix.matrix_space.MatrixSpace
without importing it. It might work by accident, but it's not good. Either you explicitlyimport sage.matrix.matrix_space
or you do something likefrom sage.matrix.matrix_space import MatrixSpace
and then useMatrixSpace
.
 Same comment for
sage.matrix.constructor
.
 Same comment for
sage.rings.power_series_ring
insrc/sage/rings/power_series_ring_element.pyx
comment:7 Changed 4 years ago by
About 1, ok
About 2,3,4:
I found this solution to break circular imports. If you think you can do better, please do. This ticket is very low on my priority list. If you feel like helping towards python3, I would appreciate any contribution to the discussion on why building with SAGE_PYTHON3 currently fails:
https://groups.google.com/forum/#!topic/sagedevel/FqR37Fu3tY
comment:8 Changed 4 years ago by
 Commit changed from 59237752defce7bffd861e80591da3246ec32d09 to 7113b26fdcc440dbf4c2d6c94c0a08d40d963e42
comment:9 Changed 4 years ago by
 Status changed from needs_work to needs_review
should be better now
comment:10 Changed 4 years ago by
 Reviewers set to Jeroen Demeyer, André Apitzsch
 Status changed from needs_review to positive_review
LGTM.
comment:11 Changed 4 years ago by
 Branch changed from u/chapoton/23068 to 7113b26fdcc440dbf4c2d6c94c0a08d40d963e42
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
more absolute imports in pyx files