Opened 10 years ago

Closed 10 years ago

# is_maximal is broken

Reported by: Owned by: jhpalmieri AlexGhitza major sage-4.7 algebra maximal ideal sage-4.7.alpha5 John Palmieri Simon Spicer N/A

### Description

The method "is_maximal" in sage/rings/ideal.py is broken:

```sage: R = IntegerModRing(8)
sage: R.principal_ideal(0).is_maximal()
True
sage: R.principal_ideal(2).is_maximal()
True
sage: R.principal_ideal(4).is_maximal()
True
```

```TODO: Make self.is_maximal() work! Write this code!
```

but we need a trac ticket for this. The comments in the code are not right, either:

```        kd = self.ring().krull_dimension()
if kd == 0: # every non-trivial ideal is maximal
```

This appears to be false, as the example above (Z/8Z) illustrates. It would be true if the ring were an integral domain (because Krull dimension 0 + integral domain = field). Alternatively, it would be true if the ideal were prime, but we have limited primality testing right now, so this is not the best way to go: with R as above, `R.principal_ideal(2).is_prime()` raises a `NotImplementedError`. Next:

```        elif kd == 1 and self.ring().is_integral_domain(): # every nontrivial ideal is maximal
return self.is_prime()
```

The comment should say "every nontrivial prime ideal is maximal, so the code is right but the comment isn't.

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

• Status changed from new to needs_review

Here's a patch: apply

• trac_10934-is-maximal.patch

All tests pass for me on sage.math.

### comment:2 Changed 10 years ago by spice

• Authors set to John Palmieri
• Reviewers set to Simon Spicer
• Status changed from needs_review to positive_review

All tests pass for me too, and the code behaves as intended on all the examples I could think of. Positive review.

Since this is only a partial implementation of is_maximal() i.e. only for quotient rings of `ZZ`, should we open a separate ticket for implementing this over general rings? Or does such a ticket already exist?

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

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