Opened 7 years ago
Closed 7 years ago
#18147 closed defect (fixed)
Rank of int matrix, documentation typo
Reported by: | jmantysalo | Owned by: | |
---|---|---|---|
Priority: | trivial | Milestone: | sage-6.7 |
Component: | linear algebra | Keywords: | |
Cc: | vdelecroix | Merged in: | |
Authors: | Jori Mäntysalo | Reviewers: | Frédéric Chapoton, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | fc49c4f (Commits, GitHub, GitLab) | Commit: | fc49c4f998eab90956699da5e523ce7f36c2c60a |
Dependencies: | Stopgaps: |
Description (last modified by )
Output section of rank()
contains input. Also Matrix.random(ZZ, 3).rank(algorithm='cat-says-meow')
gives no error message.
Change History (13)
comment:1 Changed 7 years ago by
- Branch set to u/jmantysalo/rank_of_int_matrix__documentation_typo
comment:2 Changed 7 years ago by
- Commit set to 91cb99a1854dd5efcace52d1522d44de0c474e2e
- Description modified (diff)
- Status changed from new to needs_review
comment:3 Changed 7 years ago by
- Cc vdelecroix added
I do not like this extra if. Could you rather change the code to:
if algorithm == 'flint': ... elif algorithm == 'modp': ... elif algorithm == 'linbox': ... else: raise ValueError
comment:4 Changed 7 years ago by
See the comment line
# Algorithm is 'linbox' or detecting full rank didn't work . . .
This can be done, but I think that it would make code a little unclear.
comment:5 follow-up: ↓ 6 Changed 7 years ago by
- Status changed from needs_review to needs_work
What's with the changes in indendation?
comment:6 in reply to: ↑ 5 Changed 7 years ago by
Replying to jdemeyer:
What's with the changes in indendation?
The line had 11 spaces. I have thinked that Python should have 4n
spaces. It's not at all the only line like that, see
egrep '^ {4}* {1,3}[^ ]' src/sage/matrix/matrix_integer_dense.pyx
comment:7 follow-up: ↓ 8 Changed 7 years ago by
- Status changed from needs_work to needs_review
If my explanation of spaces is good enought, maybe this can be closed?
comment:8 in reply to: ↑ 7 Changed 7 years ago by
- Status changed from needs_review to needs_work
Replying to jmantysalo:
If my explanation of spaces is good enough
No, it's not good enough. I still don't understand why you are changing the indendation on some lines from correct to wrong (please look at the diff!)
comment:9 Changed 7 years ago by
- Branch changed from u/jmantysalo/rank_of_int_matrix__documentation_typo to public/ticket/18147
- Commit changed from 91cb99a1854dd5efcace52d1522d44de0c474e2e to fc49c4f998eab90956699da5e523ce7f36c2c60a
comment:10 Changed 7 years ago by
- Reviewers set to Frédéric Chapoton
- Status changed from needs_work to needs_review
comment:11 Changed 7 years ago by
- Reviewers changed from Frédéric Chapoton to Frédéric Chapoton, Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:12 Changed 7 years ago by
- Milestone changed from sage-6.6 to sage-6.7
comment:13 Changed 7 years ago by
- Branch changed from public/ticket/18147 to fc49c4f998eab90956699da5e523ce7f36c2c60a
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Fixed doc typo; added check for parameter 'algorithm'.