Opened 12 years ago

Closed 12 years ago

# Add a factory of finite/infinite enumerated set (with categories) defines by Range(begin, end, step)

Reported by: Owned by: nborie nborie major sage-4.4 combinatorics range, categories, set, integer sage-combinat sage-4.4.alpha0 Nicolas Borie Florent Hivert N/A

A general tool to define:

The idea is : Range(begin, end, step) returns a proper finite/infinite set with categories of the defined set as you think...

(12:37:41) hivert: For example but not only: Range(5, 21321312, 3)...

(12:38:09) hivert: Range(0, Infinity, 1) <=> NonNegativeIntegers?

(12:38:18) hivert: Range(1, Infinity, 1) <=> PositiveIntegers?

### comment:1 Changed 12 years ago by nthiery

+1!

That will be useful for NCSF too (see the beginning of sage/combinat/ncsf/categories.py).

Just mind the 's': PositiveIntegers?

Let me guess: TransitiveGroup?(0,1) does not work, right? Maybe it should actually?

### comment:2 Changed 12 years ago by nborie

• Description modified (diff)
• Keywords range categories set added; positive removed
• Summary changed from Add a set of Positive Integer with categories (and factor the code with NonNegativeInteger) to Add a factory of finite/infinite enumerated set (with categories) defines by Range(begin, end, step)

### comment:3 Changed 12 years ago by nborie

• Description modified (diff)

### comment:4 Changed 12 years ago by nborie

• Description modified (diff)

### comment:5 follow-up: ↓ 6 Changed 12 years ago by nborie

Hello Nicolas, will you set a +2 with the new description ?

### comment:6 in reply to: ↑ 5 ; follow-up: ↓ 7 Changed 12 years ago by nthiery

Hello Nicolas, will you set a +2 with the new description ?

Yes. I just spent 5 minutes discussing about this with Florent over the phone. In the end, we vote for IntegerRange?(a,b, step=c), and NonNegativeIntegers? / PositiveIntegers? being subclasses (to add further properties; like the fact that they are semigroups/monoids/semirings/....).

### comment:7 in reply to: ↑ 6 Changed 12 years ago by saliola

+1 for `IntegerRange` instead of `Range`.

### comment:8 Changed 12 years ago by nborie

So guys,

I add a patch with a first implementation...

Questions:

• First, if you find a language mistake, please tell me.
• What should I keep, what should I drop in NonNegativeIntegers? ?
• Does IntegerRange? need an _element_constructor_ method ?
• Any comment ....

### comment:9 Changed 12 years ago by nborie

The contains function is false. It works only for IntegerRange?(-Infinity,Infinity,a,b)...

### comment:10 follow-up: ↓ 11 Changed 12 years ago by nborie

Ok, after think a while...

I will update this code and give IntegerRange? the behavior of range() if and only if someone (else than me) can give me an empty set with categories which can pass the tests....

```sage: TestSuite(Set([])).run()
The following tests failed: _test_an_element, _test_category, _test_elements, _test_some_elements
```

I need this feature for coherence!! Python range easily give out an empty list like:

```sage: range(1,20,-1)
[]
sage: range(20,1,1)
[]
```

Adding - and + Infinity, we will have a lot of arguments which will build an emptyset. Currently, I did not find a good empty set with categories.

My code does not allow negative step and check that begin < end to avoid the empty case.

On other hand, my current status is: Benchmarks for my PhD Thesis --> add number_of_transitive_group --> Add enumerated set of TransitiveGroups?() --> Add PositiveInteger? --> Add IntegerRange? feature --> MAX DEPTH OF RECURSION...

As this EmptySet? (with categories) is an empty thing, I have an idea of who can implemented that ?

### comment:11 in reply to: ↑ 10 Changed 12 years ago by hivert

I will update this code and give IntegerRange? the behavior of range() if and only if someone (else than me) can give me an empty set with categories which can pass the tests....

```sage: TestSuite(Set([])).run()
The following tests failed: _test_an_element, _test_category, _test_elements, _test_some_elements
```

For consintency of the category you should use an `EnumeratedSet` rather than a `Set`:

```sage: f = FiniteEnumeratedSet([])
sage: TestSuite(f).run()
[...]
The following tests failed: _test_an_element, _test_elements, _test_some_elements
```

You can't expect to get anything better than that with the current specification of sets: All those three tests are buggy in the sense that they suppose that there is at least one element in the set. So forget about it and don't run TestSuite? on an empty set until we fix this.

### comment:12 Changed 12 years ago by nborie

• Status changed from new to needs_review

This last version begin to be final...

I didn't manage to attach PositiveInteger? to the reference manual. The docbuild was always saying : UNABLE TO FIND THE MODULE. Even after 3421 checks, I didn't succeed...

### comment:13 Changed 12 years ago by hivert

Hi Nicolas,

I added a review patch which

• Improves the doc
• temporarily solve the handling of empty sets

Except for those issues your patch is good,

Florent

### comment:14 Changed 12 years ago by hivert

I had a second though and decided that `_from_integer_` was a little overkill. I removed it.

Cheers,

Florent

### comment:15 Changed 12 years ago by nborie

Hi Florent,

I am very Ok with your changes! To complete this review, please check the 3 hyperlinks I just fixed in positive_integers.py and non_negative_integers.py. That's will be a very easy review I think.

Patchs to be applied :

trac_8519_Range_factory-nb.patch trac_8519_Range_factory-review-fh.2.patch trac_8519_Range_factory-review2-nb.patch

Thanks for all.

### comment:16 Changed 12 years ago by hivert

• Authors set to Nicolas Borie
• Reviewers set to Florent Hivert
• Status changed from needs_review to positive_review

Patch looks good, ready to go.

Florent

### comment:17 follow-up: ↓ 18 Changed 12 years ago by nborie

My last comment was not well formatted, so for the release manager, please apply :

• trac_8519_Range_factory-nb.patch
• trac_8519_Range_factory-review-fh.2.patch
• trac_8519_Range_factory-review2-nb.patch

Thanks for all. Nicolas.

### comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 12 years ago by hivert

My last comment was not well formatted, so for the release manager, please apply :

• trac_8519_Range_factory-nb.patch
• trac_8519_Range_factory-review-fh.2.patch
• trac_8519_Range_factory-review2-nb.patch

Reviewing this I noticed that we forgot to mention that this depends on #8524

Florent

### comment:19 in reply to: ↑ 18 ; follow-ups: ↓ 20 ↓ 21 Changed 12 years ago by jhpalmieri

Reviewing this I noticed that we forgot to mention that this depends on #8524

In what way does it depend on it? I've merged the three patches into a prototype for Sage 4.4.alpha0, and all tests pass. Do I need to back that change out now? Or can someone review #8524 quickly (and thoroughly)?

### comment:20 in reply to: ↑ 19 Changed 12 years ago by nthiery

Reviewing this I noticed that we forgot to mention that this depends on #8524

In what way does it depend on it? I've merged the three patches into a prototype for Sage 4.4.alpha0, and all tests pass. Do I need to back that change out now? Or can someone review #8524 quickly (and thoroughly)?

Done (well, almost; Florent just need to check my changes). The dependency is functional, not syntactical. I would have expected the tests to fail without #8524! Maybe the problem that was encountered and triggered the writting of #8524 did not end up being doctested, which would be bad!

### comment:21 in reply to: ↑ 19 Changed 12 years ago by hivert

Reviewing this I noticed that we forgot to mention that this depends on #8524

In what way does it depend on it? I've merged the three patches into a prototype for Sage 4.4.alpha0, and all tests pass. Do I need to back that change out now? Or can someone review #8524 quickly (and thoroughly)?

My mistake ! Forget about my comment.

### comment:22 Changed 12 years ago by jhpalmieri

• Merged in set to sage-4.4.alpha0
• Resolution set to fixed
• Status changed from positive_review to closed

Merged in 4.4.alpha0:

• trac_8519_Range_factory-nb.patch
• trac_8519_Range_factory-review-fh.2.patch
• trac_8519_Range_factory-review2-nb.patch
Note: See TracTickets for help on using tickets.