Opened 10 years ago

Closed 10 years ago

# no checking on getitem for Ambient space of a root system

Reported by: Owned by: VivianePons sage-combinat critical sage-4.7 combinatorics ambient space, root system, getitem, combinatorialfreemodule sage-combinat sage-4.7.alpha3 Viviane Pons Nicolas M. Thiéry N/A

### Description

When you create an ambient space of a root system, there is a getitem method. But no verification is made on the given value which implies bugs on other uses of the object.

```sage: R = RootSystem('A2')
sage: A2 = R.ambient_space()
sage: A2[0]
(0, 0, 0)
sage: list(A2[0])
[(-1, 1)]
sage: A2.zero()
(0, 0, 0)
sage: list(A2.zero())
[]
```

The getitem should only get values between 1 and 3 and raise exceptions when not. Otherwise, it allows for creating elements which don't make any sense and can bring trouble :

```sage: C = CombinatorialFreeModule(QQ,A2)
sage: C.an_element()
B[(0, 0, 0)] + 2*B[(1, 0, 0)] + B[(2, 2, 3)] + 3*B[(0, 1, 0)]
sage: elt = C(A2([0,0,0])) + 2 * C(A2([1,0,0])) + C(A2([2,2,3])) + 3 * C(A2([0,1,0]))
sage: elt
B[(0, 0, 0)] + 2*B[(1, 0, 0)] + B[(2, 2, 3)] + 3*B[(0, 1, 0)]
sage: elt == C.an_element()
False
```

Indeed, the getitem is used by python to iterate on the parent and the iteration is used by CombinatorialFreeModule? to create elements.

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

• Status changed from new to needs_review

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

• Reviewers set to Nicolas M. Thiéry
• Status changed from needs_review to positive_review

Following the discussion on

this is good to go (assuming the buildbot goes green which I expect)!

Thanks,

Nicolas

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

• Milestone set to sage-4.7

### comment:4 follow-up: ↓ 5 Changed 10 years ago by jdemeyer

• Status changed from positive_review to needs_work

@VivianePons?: create a `.hgrc` file as explained in http://sagemath.org/doc/developer/walk_through.html#submitting-a-change and resubmit the patch.

### comment:5 in reply to: ↑ 4 Changed 10 years ago by nthiery

@VivianePons?: create a `.hgrc` file as explained in http://sagemath.org/doc/developer/walk_through.html#submitting-a-change and resubmit the patch.

Sorry, I should have caught this. The main point is that the patch should be exported before being posted on trac. See also: http://wiki.sagemath.org/combinat/MercurialStepByStep#ExportingPatchesforusewithtrac.

the patch

### comment:6 Changed 10 years ago by VivianePons

• Status changed from needs_work to needs_review

### comment:7 Changed 10 years ago by nthiery

• Status changed from needs_review to positive_review

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

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