Sage: Ticket #12604: A patch adding .dimensions() to a matrix.
https://trac.sagemath.org/ticket/12604
<p>
This is a simple patch which gives you the dimensions of a matrix. So if M has 3 rows and 5 columns, M.dimensions() returns the tuple (3,5).
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/12604
Trac 1.1.6dsmMon, 27 Feb 2012 23:18:23 GMTstatus changed
https://trac.sagemath.org/ticket/12604#comment:1
https://trac.sagemath.org/ticket/12604#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_info</em>
</li>
</ul>
<p>
I'm not sure we can use ".dimensions()" for this, at least by itself. Our terminology is already a little weird.. in the parent <a class="missing wiki">MatrixSpace?</a>, we have:
</p>
<pre class="wiki">sage: m = Matrix(QQ, 3, 5)
sage: m
[0 0 0 0 0]
[0 0 0 0 0]
[0 0 0 0 0]
sage: parent(m)
Full MatrixSpace of 3 by 5 dense matrices over Rational Field
sage: ms = parent(m)
sage: ms.dim
ms.dimension ms.dims
sage: ms.dims()
(3, 5)
sage: ms.dimension()
15
</pre><p>
Those aren't the names I'd have chosen, but given that they're there, we shouldn't have .dims() work for the <a class="missing wiki">MatrixSpace?</a> and .dimensions() work for an element in the space.
</p>
<p>
.dimensions() is a better name, IMHO, so maybe we should make both .dimensions() and .dimension() work for the <a class="missing wiki">MatrixSpace?</a> and for a Matrix, and let .dims() be an alias for .dimensions() for backward compatibility purposes. Above my pay grade, though.
</p>
TicketwasTue, 28 Feb 2012 04:19:32 GMTstatus changed
https://trac.sagemath.org/ticket/12604#comment:2
https://trac.sagemath.org/ticket/12604#comment:2
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_review</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12604#comment:1" title="Comment 1">dsm</a>:
</p>
<blockquote class="citation">
<p>
I'm not sure we can use ".dimensions()" for this, at least by itself.
</p>
</blockquote>
<p>
I personally can not remember using or hearing the word "dimensions" to refer to the number of rows and columns of a matrix. However, I just did a google search, and this is *definitely* very standard terminology all over the place. So I think it is very sensible to add this.
</p>
<blockquote class="citation">
<p>
Our terminology is already a little weird.. in the parent <a class="missing wiki">MatrixSpace?</a>, we have:
</p>
<pre class="wiki">sage: m = Matrix(QQ, 3, 5)
sage: m
[0 0 0 0 0]
[0 0 0 0 0]
[0 0 0 0 0]
sage: parent(m)
Full MatrixSpace of 3 by 5 dense matrices over Rational Field
sage: ms = parent(m)
sage: ms.dim
ms.dimension ms.dims
sage: ms.dims()
(3, 5)
</pre></blockquote>
<p>
It would be good to have dims = dimensions as an alias for consistency. Don't just change it since that will break code.
</p>
<blockquote class="citation">
<p>
sage: ms.dimension()
15
}}}
</p>
</blockquote>
<p>
ms.dimension() is literally the dimension of the matrix space itself. This has nothing to do with "dimensions()", except that it is the product of them.
</p>
<blockquote class="citation">
<p>
Those aren't the names I'd have chosen, but given that they're there, we shouldn't have .dims() work for the <a class="missing wiki">MatrixSpace?</a> and .dimensions() work for an element in the space.
</p>
<p>
.dimensions() is a better name, IMHO, so maybe we should make both .dimensions() and .dimension() work
for the <a class="missing wiki">MatrixSpace?</a> and for a Matrix, and let .dims() be an alias for .dimensions() for backward
compatibility purposes. Above my pay grade, though.
</p>
</blockquote>
<p>
I totally agree with your conclusion. But I'm not convinced that changes to <a class="missing wiki">MatrixSpace?</a> should be part of this ticket. It could be in another ticket.
</p>
TicketwasTue, 28 Feb 2012 04:23:16 GMTattachment set
https://trac.sagemath.org/ticket/12604
https://trac.sagemath.org/ticket/12604
<ul>
<li><strong>attachment</strong>
set to <em>trac_12604.patch</em>
</li>
</ul>
<p>
apply only this patch
</p>
TicketwasTue, 28 Feb 2012 04:23:52 GMT
https://trac.sagemath.org/ticket/12604#comment:3
https://trac.sagemath.org/ticket/12604#comment:3
<p>
REFEREE REPORT:
</p>
<ol><li>It fails all the doctests:
<pre class="wiki">wstein@sage:/scratch/wstein/sage-5.0.beta5-sage.math.washington.edu-x86_64-Linux/devel/sage/sage$ sage -t matrix/matrix0.pyx
sage -t "devel/sage-main/sage/matrix/matrix0.pyx"
**********************************************************************
File "/mnt/usb1/scratch/wstein/sage-5.0.beta5-sage.math.washington.edu-x86_64-Linux/devel/sage-main/sage/matrix/matrix0.pyx", line 2028:
sage: M.dimensions()
Expected:
(2,3)
Got:
(2, 3)
**********************************************************************
File "/mnt/usb1/scratch/wstein/sage-5.0.beta5-sage.math.washington.edu-x86_64-Linux/devel/sage-main/sage/matrix/matrix0.pyx", line 2030:
sage: N.dimensions()
Expected:
(3,2)
Got:
(3, 2)
**********************************************************************
1 items had failures:
2 of 7 in __main__.example_35
***Test Failed*** 2 failures.
For whitespace errors, see the file /scratch/wstein/sage//tmp/matrix0_29905.py
[6.3 s]
</pre></li></ol><ol start="2"><li>The description in the patch should probably be "Trac <a class="closed ticket" href="https://trac.sagemath.org/ticket/12604" title="enhancement: A patch adding .dimensions() to a matrix. (closed: fixed)">#12604</a>: Added a method to a matrix which gives the dimensions."
</li></ol><ol start="3"><li>The author should be "Ben Lundell" instead of math480 student.
</li></ol><p>
I've attached a patch fixing all these issues.
</p>
TicketwasTue, 28 Feb 2012 04:24:28 GMTkeywords changed; reviewer, author set
https://trac.sagemath.org/ticket/12604#comment:4
https://trac.sagemath.org/ticket/12604#comment:4
<ul>
<li><strong>keywords</strong>
<em>Matrices</em> added; <em>Matricies</em> removed
</li>
<li><strong>reviewer</strong>
set to <em>William Stein</em>
</li>
<li><strong>author</strong>
set to <em>Benjamin Lundell</em>
</li>
</ul>
<p>
Can somebody sign off on my review patch?
</p>
TicketdsmTue, 28 Feb 2012 14:32:53 GMT
https://trac.sagemath.org/ticket/12604#comment:5
https://trac.sagemath.org/ticket/12604#comment:5
<p>
"ms.dimension() is literally the dimension of the matrix space itself. This has nothing to do with "dimensions()", except that it is the product of them."
</p>
<p>
This is a use of the word "nothing" with which I am not familiar. :-)
</p>
TicketdkrennTue, 28 Feb 2012 14:53:31 GMTstatus changed
https://trac.sagemath.org/ticket/12604#comment:6
https://trac.sagemath.org/ticket/12604#comment:6
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketdkrennTue, 28 Feb 2012 14:53:48 GMTreviewer changed
https://trac.sagemath.org/ticket/12604#comment:7
https://trac.sagemath.org/ticket/12604#comment:7
<ul>
<li><strong>reviewer</strong>
changed from <em>William Stein</em> to <em>William Stein, Daniel Krenn</em>
</li>
</ul>
TicketwasTue, 28 Feb 2012 15:33:15 GMT
https://trac.sagemath.org/ticket/12604#comment:8
https://trac.sagemath.org/ticket/12604#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/12604#comment:5" title="Comment 5">dsm</a>:
</p>
<blockquote class="citation">
<p>
"ms.dimension() is literally the dimension of the matrix space itself. This has nothing to do with "dimensions()", except that it is the product of them."
</p>
<p>
This is a use of the word "nothing" with which I am not familiar. :-)
</p>
</blockquote>
<p>
These are not the Droids you are looking for.
</p>
TicketjdemeyerTue, 28 Feb 2012 21:23:59 GMTcomponent changed
https://trac.sagemath.org/ticket/12604#comment:9
https://trac.sagemath.org/ticket/12604#comment:9
<ul>
<li><strong>component</strong>
changed from <em>PLEASE CHANGE</em> to <em>linear algebra</em>
</li>
</ul>
TicketjdemeyerSun, 04 Mar 2012 21:16:40 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/12604#comment:10
https://trac.sagemath.org/ticket/12604#comment:10
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>merged</strong>
set to <em>sage-5.0.beta7</em>
</li>
</ul>
Ticket