[with patch, positive review] Ideal() should check arguments better

[ncalexan]

sage: Ideal(3, 5)
Principal ideal (3) of Integer Ring

Misleading!

[mabshoff]

It actually does exactly what it is supposed to do in the documentation [from Ideal?]:

        Alternatively, one can also call this function with the syntax
             Ideal(gens)
        where gens is a nonempty list of generators or a single generator.

From Sage 2.9.1.1:

sage: R.<x,y> = QQ[]
sage: R
Multivariate Polynomial Ring in x, y over Rational Field
sage: R.ideal(x^2,x-y)
Ideal (x^2, x - y) of Multivariate Polynomial Ring in x, y over Rational Field
sage: Ideal([x^2,x-y])
Ideal (x^2, x - y) of Multivariate Polynomial Ring in x, y over Rational Field
sage: Ideal(x^2,x-y)
Principal ideal (x^2) of Multivariate Polynomial Ring in x, y over Rational Field

[AlexGhitza]

The patch fixes the problem pointed out by Nick, and makes Ideal() complain loudly rather than return the wrong thing in other cases.

Before:

sage: Ideal(3,5)
Principal ideal (3) of Integer Ring
sage: Ideal(ZZ,3,5)
Principal ideal (3) of Integer Ring

After:

sage: Ideal(3,5)
Principal ideal (1) of Integer Ring
sage: Ideal(ZZ,3,5)
...

<type 'exceptions.TypeError'>: coerce must be either True or False

[dmharvey]

Sorry this patch doesn't work right. With the patch I get for example:

sage: Ideal(2, 4, 6)
[...]
<type 'exceptions.TypeError'>: coerce must be either True or False

which is still very confusing.

The recursive function call is not the right way to do this. Should use the def func(*x, **kwds) idiom instead; to see an example, look at

sage: R.<x> = PolynomialRing(ZZ)
sage: R.ideal??

[malb]

state of affairs for editorial meeting 26/06/08 No movement on my end, sorry.

[mabshoff]

Change the subject line.

malb: any hope for this ticket?

Cheers,

Michael

[AlexGhitza]

I have completely rewritten the patch in a way that I think addresses the objections given above.

[malb]

Patch applies cleanly against 3.1.2 and reads good.

[mabshoff]

Merged in Sage 3.1.3.alpha2