Opened 7 years ago

Closed 7 years ago

Boolean Multivariate Ideals should not have negative dimension....

Reported by: Owned by: Bouillaguet malb minor sage-5.2 commutative algebra polybori malb, AlexanderDreyer, PolyBori sage-5.2.beta1 Charles Bouillaguet Martin Albrecht N/A

The dimension of an ideal cannot be negative (it would be mathematically incoherent). Yet, in SAGE, it is possible to create Boolean Ideals of dimension -1.....

```sage: n=11
sage: R = BooleanPolynomialRing(n, 'x')
sage: R2 = PolynomialRing(GF(2), n, 'x')
sage: I = ideal([ R(f) for f in sage.rings.ideal.Cyclic(R2, n).gens() ])
sage: I.dimension()
-1
```

In fact, all the BooleanPolynomialIdeal's should have dimension zero. Thus I suggest to overload the dimension() method to just return zero....

Agreed!

comment:2 Changed 7 years ago by Bouillaguet

• Priority changed from major to minor
• Status changed from new to needs_review

comment:3 Changed 7 years ago by malb

• Reviewers set to Martin Albrecht
• Status changed from needs_review to needs_work

Looks good except for one minor detail

```TESTS::
# check that Ticket #13155 is solved
```

should be rewritten as:

```TESTS:

Check that :trac:`13155` is solved::

sage: ...
```

So there's an empty line between `::` and the Sage tests (which I believe is required) and it uses the `:trac:`foo`` directive.

comment:4 Changed 7 years ago by Bouillaguet

• Description modified (diff)
• Status changed from needs_work to needs_review

comment:5 follow-up: ↓ 6 Changed 7 years ago by malb

Sorry for being picky but there's still an issue there. The `::` indicates that the code block follows (which is then used for doctests. You have two of those one after TESTS and one after the sentence. You need to drop the one after TESTS

comment:6 in reply to: ↑ 5 Changed 7 years ago by Bouillaguet

Sorry for being picky but there's still an issue there. The `::` indicates that the code block follows (which is then used for doctests. You have two of those one after TESTS and one after the sentence. You need to drop the one after TESTS

I will eventually learn to do this right on the first try... Patch updated.

comment:7 Changed 7 years ago by malb

• Status changed from needs_review to positive_review

all good now

comment:8 Changed 7 years ago by jdemeyer

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