Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minpoly and charpoly don't check their arguments correctly #13187

Closed
SnarkBoojum mannequin opened this issue Jun 30, 2012 · 6 comments
Closed

Minpoly and charpoly don't check their arguments correctly #13187

SnarkBoojum mannequin opened this issue Jun 30, 2012 · 6 comments

Comments

@SnarkBoojum
Copy link
Mannequin

SnarkBoojum mannequin commented Jun 30, 2012

The following fails in sage (likewise with charpoly):

k=CyclotomicField(10)
U=matrix(k, 1,1,[1])
var('t')
U.minpoly(t)

The error is in a call that the function makes to PolynomialRing :

TypeError: invalid input (Cyclotomic Field of order 10 and degree 4, t, None) to PolynomialRing function; please see the docstring for that function

Strangely, if you use :

k=CyclotomicField(10)
U=matrix(k, 1,1,[1])
var('t')
U.minpoly('t')

everything is fine.

In any case, I would expect charpoly/minpoly to complain themselves if I don't call them correctly (which isn't clear at all!) -- it's definitely not normal (and upsetting for newbies) that another function reacts.

This is with sage-5.1.beta6.

Component: linear algebra

Author: André Apitzsch

Reviewer: Robert Bradshaw

Merged: sage-5.2.beta1

Issue created by migration from https://trac.sagemath.org/ticket/13187

@SnarkBoojum SnarkBoojum mannequin added this to the sage-5.1 milestone Jun 30, 2012
@a-andre
Copy link
Contributor

a-andre commented Jun 30, 2012

Author: André Apitzsch

@a-andre
Copy link
Contributor

a-andre commented Jun 30, 2012

comment:1

Attachment: trac_13187.patch.gz

The attached patch fixes the problem and contains some code clean-up.

@SnarkBoojum
Copy link
Mannequin Author

SnarkBoojum mannequin commented Jun 30, 2012

comment:2

Eh, just moving that piece of code around is enough? Good!

Five hours between the report and the patch? Excellent!

Thanks!

@robertwb
Copy link
Contributor

robertwb commented Jul 1, 2012

comment:3

Yep, that looks like a good fix to me. Lots of style changes (improvements) in this patch as well, but that's fine.

@jdemeyer
Copy link
Contributor

jdemeyer commented Jul 1, 2012

Reviewer: Robert Bradshaw

@jdemeyer jdemeyer modified the milestones: sage-5.1, sage-5.2 Jul 1, 2012
@jdemeyer
Copy link
Contributor

jdemeyer commented Jul 7, 2012

Merged: sage-5.2.beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants