Sage: Ticket #10601: QuaternionAlgebra constructor does not work for python int
https://trac.sagemath.org/ticket/10601
<p>
<a class="missing wiki">QuaternionAlgebra?</a> constructor does not work with python int input. This is unfortunate for *.py scripts
</p>
<pre class="wiki">Quaternion Algebra (1, 1) with base ring Rational Field
sage: QuaternionAlgebra(1r,1)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
/home/luisfe/<ipython console> in <module>()
/opt/SAGE/sage-devel/local/lib/python2.6/site-packages/sage/algebras/quatalg/quaternion_algebra.pyc in QuaternionAlgebra(arg0, arg1, arg2, names)
175 b = v[1]
176 else:
--> 177 raise ValueError, "unknown input"
178
179 # QuaternionAlgebra(K, a, b)
ValueError: unknown input
sage: QuaternionAlgebra(1,1r)
Quaternion Algebra (1, 1) with base ring Rational Field
sage: QuaternionAlgebra(1r,1r)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
/home/luisfe/<ipython console> in <module>()
/opt/SAGE/sage-devel/local/lib/python2.6/site-packages/sage/algebras/quatalg/quaternion_algebra.pyc in QuaternionAlgebra(arg0, arg1, arg2, names)
175 b = v[1]
176 else:
--> 177 raise ValueError, "unknown input"
178
179 # QuaternionAlgebra(K, a, b)
ValueError: unknown input
</pre><p>
The problem seems to be the first numeric parameter. This will be easy to fix.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/10601
Trac 1.1.6spiceTue, 22 Mar 2011 19:28:41 GMTstatus changed; keywords, author set
https://trac.sagemath.org/ticket/10601#comment:1
https://trac.sagemath.org/ticket/10601#comment:1
<ul>
<li><strong>keywords</strong>
<em>QuaternionAlgebra</em> added
</li>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>author</strong>
set to <em>Simon Spicer</em>
</li>
</ul>
<p>
Fixed the <a class="missing wiki">QuaternionAlgebra?</a> constructor to accept Python ints, longs and floats as input. My code's a bit inelegant, so review/rewrite is welcome.
</p>
TicketrbeezerWed, 23 Mar 2011 00:50:18 GMT
https://trac.sagemath.org/ticket/10601#comment:2
https://trac.sagemath.org/ticket/10601#comment:2
<p>
OK, now I see that the following has always been possible:
</p>
<pre class="wiki">sage: T=QuaternionAlgebra(RR(3), RR(3))
sage: T
Quaternion Algebra (3.00000000000000, 3.00000000000000) with base ring Real Field with 53 bits of precision
</pre><p>
This looks good - some suggestions follow. Let me know, and then I'll run tests.
</p>
<ol><li> Maybe <code>is_RingElement</code> would be an improved test, as it would catch some things sooner and give a better error message?
<pre class="wiki">sage: a=PermutationGroupElement([1,2,3])
sage: QuaternionAlgebra(a, a)
<snip>
AttributeError: 'SymmetricGroup_with_category' object has no attribute 'fraction_field'
sage: is_Element(a)
<snip>
True
sage: is_RingElement(a)
False
</pre></li></ol><ol start="2"><li> I think that the error message could be more informative. Probably be good to have a doctest that raises this error (you could use the permutation example above).
<pre class="wiki">ValueError('input is not an element of a ring: {0}'.format(a))
</pre></li></ol><ol start="3"><li> It might be good practice to preface your new tests with a sentence saying they check Trac <a class="closed ticket" href="https://trac.sagemath.org/ticket/10601" title="defect: QuaternionAlgebra constructor does not work for python int (closed: fixed)">#10601</a>.
</li></ol>
TicketspiceWed, 23 Mar 2011 02:18:12 GMT
https://trac.sagemath.org/ticket/10601#comment:3
https://trac.sagemath.org/ticket/10601#comment:3
<p>
Thanks for the pointers. I've updated the patch as per your three suggestions.
</p>
TicketrbeezerWed, 23 Mar 2011 06:47:14 GMT
https://trac.sagemath.org/ticket/10601#comment:4
https://trac.sagemath.org/ticket/10601#comment:4
<p>
Simon,
</p>
<p>
Looks good and passes all long tests. Can you make just one more edit?
</p>
<pre class="wiki"># The following tests address the issues rased in trac ticket 10601
</pre><p>
can go right up in the text preceding, that seems to be the custom - ie as part of the visible documentation (and "raised" needs a letter). If you can do that, I'll just peek at the patch and we'll be done. Thanks for your work on this.
</p>
<p>
Rob
</p>
TicketspiceWed, 23 Mar 2011 16:13:24 GMTattachment set
https://trac.sagemath.org/ticket/10601
https://trac.sagemath.org/ticket/10601
<ul>
<li><strong>attachment</strong>
set to <em>trac_10601_QuaternionAlgebra_constructor.patch</em>
</li>
</ul>
<p>
Replaces previous patch
</p>
TicketspiceWed, 23 Mar 2011 16:16:06 GMT
https://trac.sagemath.org/ticket/10601#comment:5
https://trac.sagemath.org/ticket/10601#comment:5
<p>
Done. Changed to:
</p>
<pre class="wiki">Python ints, longs and floats may be passed to the ``QuaternionAlgebra(a, b)`` constructor, as may
all pairs of nonzero elements of a ring not of characteristic 2. The following tests address
the issues raised in trac ticket 10601::
sage: QuaternionAlgebra(1r,1)
Quaternion Algebra (1, 1) with base ring Rational Field
sage: QuaternionAlgebra(1,1.0r)
Quaternion Algebra (1.00000000000000, 1.00000000000000) with base ring Real Field with 53 bits of precisi\
on
</pre><p>
etc.
</p>
<p>
Apologies, I'm still a bit green when it comes to <a class="missing wiki">SageDev?</a>; still trying to get used to all the coding conventions :-)
</p>
TicketrbeezerWed, 23 Mar 2011 16:26:33 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/10601#comment:6
https://trac.sagemath.org/ticket/10601#comment:6
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Rob Beezer</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/10601#comment:5" title="Comment 5">spice</a>:
</p>
<blockquote class="citation">
<p>
Apologies, I'm still a bit green when it comes to <a class="missing wiki">SageDev?</a>; still trying to get used to all the coding conventions :-)
</p>
</blockquote>
<p>
No problem - there's a lot to keep track of, and that's what reviews are for. (And I could have been more explicit about where to put the Trac number.)
</p>
<p>
This looks real good. Positive review
</p>
TicketjdemeyerWed, 13 Apr 2011 07:43:09 GMTstatus changed; resolution, merged set
https://trac.sagemath.org/ticket/10601#comment:7
https://trac.sagemath.org/ticket/10601#comment:7
<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.alpha5</em>
</li>
</ul>
Ticket