Sage: Ticket #10595: Vector constructor fails on empty list with no ring given
https://trac.sagemath.org/ticket/10595
<pre class="wiki">sage: vector(QQ,[])
()
sage: vector([])
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
/home/sage/sage-4.6.1.rc1/devel/sage-main/<ipython console> in <module>()
/sage/sage-4.6.1.rc1/local/lib/python2.6/site-packages/sage/modules/free_module_element.so in sage.modules.free_module_element.vector (sage/modules/free_module_element.c:3229)()
/sage/sage-4.6.1.rc1/local/lib/python2.6/site-packages/sage/modules/free_module_element.so in sage.modules.free_module_element.prepare (sage/modules/free_module_element.c:3675)()
TypeError: unable to find a common ring for all elements
</pre><p>
Default ring should be <code>ZZ</code> ideally.
</p>
<hr />
<p>
Apply both
</p>
<ul><li>trac_10595-vector-constructor-empty-list.patch
</li><li>10595-referee.patch
</li></ul>en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10595
Trac 1.2Rob BeezerTue, 11 Jan 2011 23:31:18 GMTattachment set
https://trac.sagemath.org/ticket/10595
https://trac.sagemath.org/ticket/10595
<ul>
<li><strong>attachment</strong>
set to <em>trac_10595-vector-constructor-empty-list.patch</em>
</li>
</ul>
TicketRob BeezerTue, 11 Jan 2011 23:33:34 GMTstatus changed; author set
https://trac.sagemath.org/ticket/10595#comment:1
https://trac.sagemath.org/ticket/10595#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>author</strong>
set to <em>Rob Beezer</em>
</li>
</ul>
<p>
Patch depends on <a class="closed ticket" href="https://trac.sagemath.org/ticket/10537" title="#10537: defect: Fix dictionary input to sparse vector constructor (closed: fixed)">#10537</a>, <a class="closed ticket" href="https://trac.sagemath.org/ticket/10364" title="#10364: enhancement: Vector constructor improvements (closed: fixed)">#10364</a>.
</p>
TicketDima PasechnikTue, 11 Jan 2011 23:55:02 GMTstatus changed
https://trac.sagemath.org/ticket/10595#comment:2
https://trac.sagemath.org/ticket/10595#comment:2
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
</ul>
TicketJeroen DemeyerTue, 25 Jan 2011 09:23:52 GMTreviewer set
https://trac.sagemath.org/ticket/10595#comment:3
https://trac.sagemath.org/ticket/10595#comment:3
<ul>
<li><strong>reviewer</strong>
set to <em>Dmitrii Pasechnik</em>
</li>
</ul>
TicketJeroen DemeyerTue, 25 Jan 2011 16:13:07 GMTstatus changed
https://trac.sagemath.org/ticket/10595#comment:4
https://trac.sagemath.org/ticket/10595#comment:4
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
This patch gives a massive number of doctest failures.
</p>
TicketRob BeezerTue, 25 Jan 2011 23:26:06 GMT
https://trac.sagemath.org/ticket/10595#comment:5
https://trac.sagemath.org/ticket/10595#comment:5
<p>
The following existing (undocumented) behavior was broken by this patch, so needs to be reinstated or worked around. I thought this went through all a full test suite, but I guess not.
</p>
<pre class="wiki">sage: z=ComplexField().an_element()
sage: z
1.00000000000000*I
sage: vector(z)
(0.000000000000000, 1.00000000000000)
</pre>
TicketDima PasechnikWed, 26 Jan 2011 09:52:43 GMT
https://trac.sagemath.org/ticket/10595#comment:6
https://trac.sagemath.org/ticket/10595#comment:6
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:5" title="Comment 5">rbeezer</a>:
</p>
<blockquote class="citation">
<p>
The following existing (undocumented) behavior was broken by this patch, so needs to be reinstated or worked around. I thought this went through all a full test suite, but I guess not.
</p>
</blockquote>
<pre class="wiki"> sage: z=ComplexField().an_element()
sage: z
1.00000000000000*I
sage: vector(z)
(0.000000000000000, 1.00000000000000)
</pre><p>
jeez, what an awful kludge...
</p>
TicketRob BeezerThu, 27 Jan 2011 18:20:17 GMT
https://trac.sagemath.org/ticket/10595#comment:7
https://trac.sagemath.org/ticket/10595#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:6" title="Comment 6">dimpase</a>:
</p>
<blockquote class="citation">
<p>
jeez, what an awful kludge...
</p>
</blockquote>
<p>
Agreed. Worse that it was not documented, I think. And it causes about 80 doctest failures in one module, so there is probably no getting rid of it. So I'll figure out how it was happening in the first place, return the behavior, and <em>document</em> it.
</p>
<p>
Rob
</p>
TicketDima PasechnikFri, 28 Jan 2011 04:03:13 GMT
https://trac.sagemath.org/ticket/10595#comment:8
https://trac.sagemath.org/ticket/10595#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:7" title="Comment 7">rbeezer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:6" title="Comment 6">dimpase</a>:
</p>
<blockquote class="citation">
<p>
jeez, what an awful kludge...
</p>
</blockquote>
<p>
Agreed. Worse that it was not documented, I think. And it causes about 80 doctest failures in one module, so there is probably no getting rid of it. So I'll figure out how it was happening in the first place, return the behavior, and <em>document</em> it.
</p>
</blockquote>
<p>
IMHO, if this "feature" is limited to complex numbers only, it better be eradicated. It should be easy to track down and fix the library code that uses it.
</p>
<p>
Dima
</p>
<blockquote class="citation">
<p>
Rob
</p>
</blockquote>
TicketRob BeezerFri, 28 Jan 2011 05:54:33 GMTstatus changed
https://trac.sagemath.org/ticket/10595#comment:9
https://trac.sagemath.org/ticket/10595#comment:9
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_info</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:8" title="Comment 8">dimpase</a>:
</p>
<blockquote class="citation">
<p>
IMHO, if this "feature" is limited to complex numbers only, it better be eradicated. It should be easy to track down and fix the library code that uses it.
</p>
</blockquote>
<p>
I agree - in principle. But I also have my hands full with some other changes in behavior. ;-)
</p>
<p>
I'll do some tests soon to gauge the magnitude of the chore and we can go from there.
</p>
<p>
Rob
</p>
TicketRob BeezerFri, 28 Jan 2011 06:54:14 GMTstatus changed
https://trac.sagemath.org/ticket/10595#comment:10
https://trac.sagemath.org/ticket/10595#comment:10
<ul>
<li><strong>status</strong>
changed from <em>needs_info</em> to <em>needs_work</em>
</li>
</ul>
<p>
Here's the extent of converting complex numbers to vectors via the vector() constructor. Of course many of the failures are consequences of some earlier failure.
</p>
<pre class="wiki"> sage -t -force_lib pha2/devel/sage/doc/en/bordeaux_2008/elliptic_curves.rst # 3 doctests failed
sage -t -force_lib pha2/devel/sage/sage/crypto/mq/sbox.py # 5 doctests failed
sage -t -force_lib pha2/devel/sage/sage/crypto/mq/sr.py # 1 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/ell_egros.py # 8 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/period_lattice.py # 61 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/modular_parametrization.py # 8 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/ell_rational_field.py # 30 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/constructor.py # 7 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/heegner.py # 81 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/elliptic_curves/ell_point.py # 16 doctests failed
sage -t -force_lib pha2/devel/sage/sage/schemes/generic/toric_divisor.py # 36 doctests failed
sage -t -force_lib pha2/devel/sage/sage/plot/colors.py # 1 doctests failed
sage -t -force_lib pha2/devel/sage/sage/algebras/quatalg/quaternion_algebra.py # 3 doctests failed
sage -t -force_lib pha2/devel/sage/sage/geometry/cone.py # 1 doctests failed
</pre>
TicketDima PasechnikSun, 30 Jan 2011 03:47:06 GMT
https://trac.sagemath.org/ticket/10595#comment:11
https://trac.sagemath.org/ticket/10595#comment:11
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:9" title="Comment 9">rbeezer</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:8" title="Comment 8">dimpase</a>:
</p>
<blockquote class="citation">
<p>
IMHO, if this "feature" is limited to complex numbers only, it better be eradicated. It should be easy to track down and fix the library code that uses it.
</p>
</blockquote>
<p>
I agree - in principle. But I also have my hands full with some other changes in behavior. ;-)
</p>
<p>
I'll do some tests soon to gauge the magnitude of the chore and we can go from there.
</p>
</blockquote>
<p>
how about the following:
</p>
<p>
make vector(blah) check whether blah.vector() can be called, and call it, if it is there.
</p>
<p>
add vector() member function to the corresponding complex number class.
</p>
<p>
Dima
</p>
<blockquote class="citation">
<p>
Rob
</p>
</blockquote>
TicketRob BeezerSun, 30 Jan 2011 05:22:13 GMT
https://trac.sagemath.org/ticket/10595#comment:12
https://trac.sagemath.org/ticket/10595#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10595#comment:11" title="Comment 11">dimpase</a>:
</p>
<blockquote class="citation">
<p>
how about the following:
</p>
<p>
make vector(blah) check whether blah.vector() can be called, and call it, if it is there.
</p>
<p>
add vector() member function to the corresponding complex number class.
</p>
</blockquote>
<p>
Yes, that's nice. Or even just call the member function every place it is needed, without adding more to the general vector constructor. That'd still require a fair bit of editing, but it would be mindless.
</p>
<p>
Rob
</p>
TicketJohn PalmieriMon, 21 Mar 2011 21:34:08 GMTattachment set
https://trac.sagemath.org/ticket/10595
https://trac.sagemath.org/ticket/10595
<ul>
<li><strong>attachment</strong>
set to <em>10595-referee.patch</em>
</li>
</ul>
<p>
apply on top of other patch
</p>
TicketJohn PalmieriMon, 21 Mar 2011 21:38:28 GMTstatus changed
https://trac.sagemath.org/ticket/10595#comment:13
https://trac.sagemath.org/ticket/10595#comment:13
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Here's a patch which seems to fix the problem for me. I think the issue was this: some Sage objects (like complex numbers) can be converted to type "Sequence", and then those can be converted to vectors. But if those objects had no "len", then the patch would cause problems, so the "referee" patch tries to call "len", but ignores TypeErrors.
</p>
<p>
By the way, the "vector" function already checks to see if <code>v._vector_()</code> is defined, so another option would be to define a <code>_vector_</code> method for complex numbers. This would fix many of the doctest errors, but not all of them, because some of them pass other objects (like generators) to "vector".
</p>
TicketJohn PalmieriMon, 21 Mar 2011 21:39:32 GMTdescription changed
https://trac.sagemath.org/ticket/10595#comment:14
https://trac.sagemath.org/ticket/10595#comment:14
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/10595?action=diff&version=14">diff</a>)
</li>
</ul>
<p>
To the patchbot: Apply both
</p>
<ul><li>trac_10595-vector-constructor-empty-list.patch
</li><li>10595-referee.patch
</li></ul>
TicketRob BeezerMon, 21 Mar 2011 23:18:20 GMTstatus, reviewer, author changed
https://trac.sagemath.org/ticket/10595#comment:15
https://trac.sagemath.org/ticket/10595#comment:15
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
changed from <em>Dmitrii Pasechnik</em> to <em>Dmitrii Pasechnik, John Palmieri, Rob Beezer</em>
</li>
<li><strong>author</strong>
changed from <em>Rob Beezer</em> to <em>Rob Beezer, John Palmieri</em>
</li>
</ul>
<p>
Referee patch does the job. Thanks, John. Dima and John have checked off on the original patch, and I'll check off on the referee patch.
</p>
<p>
Complex number behavior, and a bit more, will be documented on <a class="closed ticket" href="https://trac.sagemath.org/ticket/10977" title="#10977: defect: Document vector constructor behavior (closed: fixed)">#10977</a>.
</p>
TicketJeroen DemeyerThu, 24 Mar 2011 20:39:28 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/10595#comment:16
https://trac.sagemath.org/ticket/10595#comment:16
<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-4.7.alpha3</em>
</li>
</ul>
Ticket