Opened 11 years ago

Closed 11 years ago

# Ideal with no generators for univariate polynomial rings

Reported by: Owned by: mraum malb critical sage-4.7.1 commutative algebra sage-4.7.1.alpha1 Martin Raum, Jeroen Demeyer John Palmieri, Martin Raum N/A

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 11139_alternative.patch.

### comment:1 Changed 11 years ago by jhpalmieri

• Authors set to Martin Raum
• Reviewers set to John Palmieri
• Status changed from new to needs_review

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.

### comment:2 Changed 11 years ago by jhpalmieri

• Description modified (diff)

### comment:3 Changed 11 years ago by jdemeyer

• Authors changed from Martin Raum to Martin Raum, Jeroen Demeyer
• Description modified (diff)

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

### comment:4 follow-up: ↓ 5 Changed 11 years ago by mraum

• Status changed from needs_review to needs_info

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?

### comment:5 in reply to: ↑ 4 Changed 11 years ago by jdemeyer

• Status changed from needs_info to needs_review

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.

### Changed 11 years ago by jdemeyer

Alternative patch, apply only this

### comment:6 Changed 11 years ago by jdemeyer

• Description modified (diff)

Patch tested, all tests passed.

### comment:7 Changed 11 years ago by jdemeyer

• Milestone changed from sage-4.7 to sage-4.7.1
• Priority changed from blocker to critical

### comment:8 Changed 11 years ago by jdemeyer

• Description modified (diff)

Any reviewers?

### comment:9 Changed 11 years ago by mraum

• Reviewers changed from John Palmieri to John Palmieri, Martin Raum
• Status changed from needs_review to positive_review

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.

### comment:10 Changed 11 years ago by jdemeyer

• Merged in set to sage-4.7.1.alpha1
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.