Ideal with no generators for univariate polynomial rings

[sagetrac-mraum]

Consider the following (Sage 4.7alpha1) :

sage: P.<a> = QQ[]
sage: P.ideal([])
Booom!

The issue is twofold: The ideal method in rings/rings.py is called and does not accept an empty list. But also the univariate polynomial rings should call a specialized method just like the multivariate ones do.

It worked in earlier versions (like 4.2 or so) and is a construction that easily comes up as a corner case.

Apply attachment: 11139_alternative.patch.

Component: commutative algebra

Author: Martin Raum, Jeroen Demeyer

Reviewer: John Palmieri, Martin Raum

Merged: sage-4.7.1.alpha1

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

[sagetrac-mraum]

Attachment: trac-11139-ideals_with_no_generators.patch.gz

[jhpalmieri]

Author: Martin Raum

[jhpalmieri]

Reviewer: John Palmieri

[jhpalmieri]

comment:1

I'm happy with this. Here's a new patch with a few doctests illustrating that the problem has been fixed.

I'm going to mark this as "needs review"; it wouldn't hurt to have someone else look it over.

[jhpalmieri]

Attachment: trac-11139-ideals_with_no_generators.v2.patch.gz

[jdemeyer]

Changed author from Martin Raum to Martin Raum, Jeroen Demeyer

[jdemeyer]

comment:3

What do you think of this alternative patch? IMHO, it has better logic.

[sagetrac-mraum]

comment:4

I think that indeed the logic is much better. Though it might be confusing to beginners. But this improvement brings my attention to a problem, that has been there before:

R.<x> = QQ[]
R.ideal((x for _ in range(2)))

This is an instance of GeneratorType, that is called with len. So either we exclude the GeneratorType (no change to the situtation before) or correct it. Also, mind that a ring element might be an instance of GeneratorType. In that (very exceptional) situation, the new logic would cause trouble.

Any experience with similar code at other places in Sage?

[jdemeyer]

comment:5

Replying to @sagetrac-mraum:

This is an instance of GeneratorType, that is called with len. So either we exclude the GeneratorType (no change to the situtation before) or correct it.

I would say not to check for GeneratorType. I have added a new patch for this. I still have to test it thoroughly.

[jdemeyer]

Alternative patch, apply only this

[jdemeyer]

comment:6

Attachment: 11139_alternative.patch.gz

Patch tested, all tests passed.

[jdemeyer]

comment:8

Any reviewers?

[sagetrac-mraum]

comment:9

I think, since I haven't written this ultimate patch, I am a legitimate reviewer. All tests pass and also the patch bot does not complain. Although we have changed the logic, backward compatibility is preserved.

[sagetrac-mraum]

Changed reviewer from John Palmieri to John Palmieri, Martin Raum

[jdemeyer]

Merged: sage-4.7.1.alpha1