Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make a good use of the EmptySetError in EnumeratedSets and Parent #10651

Closed
sagetrac-nborie mannequin opened this issue Jan 18, 2011 · 21 comments
Closed

Make a good use of the EmptySetError in EnumeratedSets and Parent #10651

sagetrac-nborie mannequin opened this issue Jan 18, 2011 · 21 comments

Comments

@sagetrac-nborie
Copy link
Mannequin

sagetrac-nborie mannequin commented Jan 18, 2011

As Florent Hivert gave extremely useful features about empty things, we should use it. The Following should returns an EmptySetError

sage: S = Set([])
sage: S.an_element()
---------------------------------------------------------------------------
StopIteration                             Traceback (most recent call last)

...


StopIteration: 
sage: 

Also, make the EmptySetError being documented already in the structure of Parent. Improve the documentation of an_element and _an_element_ in Parent.

CC: @sagetrac-sage-combinat

Component: categories

Keywords: days28, empty set, EmptySetError

Author: Nicolas Borie, Nicolas M. Thiéry

Reviewer: Paul Zimmermann, Robert Bradshaw, Nicolas Borie, Nicolas M. Thiéry

Merged: sage-4.7.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/10651

@sagetrac-nborie sagetrac-nborie mannequin added this to the sage-4.6.2 milestone Jan 18, 2011
@sagetrac-nborie sagetrac-nborie mannequin self-assigned this Jan 18, 2011
@slel
Copy link
Member

slel commented Jan 18, 2011

Changed keywords from empty set, EmptySetError to days28, empty set, EmptySetError

@robertwb
Copy link
Contributor

comment:3

Technically, some_elements should return an iterable, not an iterator, so you can iterate over it more than once. The except clause should be changed to return [], not return iter([]), though with the empty case the behavior is the same.

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Jan 27, 2011

comment:4

Good point. I will ask today Nicolas what does he think about that... I don't really know the specifications for the Test Suite and how deal exactly with such corner case.
Florent did this with FiniteEnumeratedSet (The user know a priori that the set is finite...)

sage: FiniteEnumeratedSet([])
{}
sage: FiniteEnumeratedSet([]).some_elements()
<generator object _some_elements_from_iterator at 0xb433464>

I will give an answer shortly for that.

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Jan 31, 2011

comment:5

As this ticket fix a corner case and try to increase of the coherence for the code about sets, it have to verify all the specifications. Thanks you very much Robert for pointing this. It now return []. Even the behavior is the same, I don't want to fix a mathematical corner case by introducing a Spec corner case...

It should be ready for review now.

@zimmermann6
Copy link

comment:6

I confirm the problem is fixed, and all doctests still pass.
Thus I give a positive review.

Paul

@zimmermann6
Copy link

Reviewer: Paul Zimmermann

@zimmermann6
Copy link

Author: Nicolas Borie

@jdemeyer
Copy link

jdemeyer commented Feb 8, 2011

comment:7

I think you should add an example which shows the EmptySetError.

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Feb 8, 2011

comment:8

I just had two tests... Especially one which returns the EmptySetError... Thanks all for your comments and suggestions.

@zimmermann6
Copy link

comment:9

after thought, the on-line documentation of S.an_element should mention that if S is
empty, an EmptySetError is returned.

Paul

@zimmermann6
Copy link

Work Issues: update documentation

@robertwb
Copy link
Contributor

comment:11

Looking good now.

@jdemeyer jdemeyer modified the milestones: sage-4.6.2, sage-4.7 Feb 18, 2011
@jdemeyer
Copy link

comment:13

You should change the commit message of the patch (using hg qrefresh -e) to something sensible, making sure the ticket number appears on the first line of the commit message.

@jdemeyer
Copy link

Changed work issues from update documentation to none

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Mar 21, 2011

comment:14

Attachment: trac_10651_fix_an_element_from_iterator-nb.patch.gz

In order to fix more things in the same time, I update the description of this ticket. I folded the NT'reviewer from the sage-combinat queue, then I uploaded the patch with a better commit message.

I am very OK with the changes in the Parent structure from Nicolas Thiéry. I give a positive review on all changes touching the corresponding file. The first part of the patch still comes from me, so no review from me.

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Mar 21, 2011

Changed reviewer from Paul Zimmermann to Paul Zimmermann, Robert Bradshaw, Nicolas Borie

@sagetrac-nborie

This comment has been minimized.

@sagetrac-nborie
Copy link
Mannequin Author

sagetrac-nborie mannequin commented Mar 21, 2011

Changed author from Nicolas Borie to Nicolas Borie, Nicolas M. Thiéry

@sagetrac-nborie sagetrac-nborie mannequin changed the title Small fix in _an_element_from_iterator in EnumeratedSets Make a good use of the EmptySetError in EnumeratedSets and Parent Mar 21, 2011
@nthiery
Copy link
Contributor

nthiery commented Mar 21, 2011

comment:15

And positive review on your part. Good to go!

@nthiery
Copy link
Contributor

nthiery commented Mar 21, 2011

Changed reviewer from Paul Zimmermann, Robert Bradshaw, Nicolas Borie to Paul Zimmermann, Robert Bradshaw, Nicolas Borie, Nicolas M. Thiéry

@jdemeyer
Copy link

Merged: sage-4.7.alpha3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants