Opened 8 years ago
Last modified 6 years ago
#13564 needs_info enhancement
Improve diagonal from matrix/matrix2.pyx
Reported by: | r.gaia.cs | Owned by: | jason, was |
---|---|---|---|
Priority: | major | Milestone: | sage-6.4 |
Component: | linear algebra | Keywords: | matrix, diagonal |
Cc: | Merged in: | ||
Authors: | Raniere Silva | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
The method diagonal could have a parameter so that it can return any diagonal parallel to the main diagonal.
Apply only trac_13564-diagonal.patch to devel/sage
.
Attachments (2)
Change History (13)
Changed 8 years ago by
comment:1 Changed 8 years ago by
- Status changed from new to needs_review
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
- Description modified (diff)
comment:4 Changed 8 years ago by
Have to force the patchbot :( Patchbot apply trac_13564-diagonal.patch
comment:5 Changed 8 years ago by
Some comments:
- Should we be raising an error or simply returning an empty list if the
d
for thed
-th diagonal is provided outside the range of the number of rows or columns? For instance, the empty matrix is returning an empty diagonal. Also, Mathematica and Matlab silently return empty matrices. Octave raises an error. Numpy gives an empty matrix untilnrows()
orncols()
and then raises an error. - If it seems more proper to raise an error, the error string could be written like the statements below. Note that
d
is always less than and never equal tonrows()
orncols()
.The value `d` should be less than the number of columns in the matrix. The value `-d` should be less than the number of rows of the matrix.
EDIT: The numpy error seems to stem from its refusal to create arrays which have one dimension negative. So, 0x1 array is ok, but not -1x1. The error is probably not from the diagonal code itself.
comment:6 follow-up: ↓ 7 Changed 8 years ago by
- Cc sage-devel added
- Status changed from needs_review to needs_info
I am changing this to "needs_info" for now as it seems to me that, there is still some design decisions to make. I am adding cc: sage-devel so that this could be discussed.
comment:7 in reply to: ↑ 6 Changed 8 years ago by
- Cc sage-devel removed
Replying to knsam:
I am changing this to "needs_info" for now as it seems to me that, there is still some design decisions to make. I am adding cc: sage-devel so that this could be discussed.
There is no user called "sage-devel". If you want to "cc sage-devel" then you need to put a post to the sage-devel mailing list. I was waiting for a reply from the author for clarification on the current implementation - thanks for setting this to needs_info. However, the author seems inactive since last November.
comment:8 Changed 7 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:9 Changed 7 years ago by
- Milestone changed from sage-6.1 to sage-6.2
comment:10 Changed 7 years ago by
- Milestone changed from sage-6.2 to sage-6.3
comment:11 Changed 6 years ago by
- Milestone changed from sage-6.3 to sage-6.4
Hello, this comment is with respect to the statement "- Unknown: No author specified in the file from 2012-10-03" in your patch. The authors are mentioned in the beginning of the file.
As such, I think typically no authors are added at the top unless some major change is effected. The author names and history is preserved in mercurial anyway. You can run
sage -hg blame filename
to see which revision modified which line of the filefilename
. The revision number can then be used to pinpoint the author.