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

Added is_iterator method and fixes sage to use it. #7398

Closed
nthiery opened this issue Nov 5, 2009 · 11 comments
Closed

Added is_iterator method and fixes sage to use it. #7398

nthiery opened this issue Nov 5, 2009 · 11 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented Nov 5, 2009

The following mantra occurs at three places in Sage's code to test whether v is an iterator:

     if hasattr(v, 'next'):

This is not quite correct since some sage objects have a next method without being iterable, or with a different semantic.

Let me quote python's doc:

The iterator objects themselves are required to support the following two methods, which together form the iterator protocol:

iterator.iter()::

 Return the iterator object itself. This is required to allow both containers and iterators to be used with the for and in statements. This method corresponds to the tp_iter slot of the type structure for Python objects in the Python/C API.

iterator.next()::

Return the next item from the container. If there are no further items, raise the StopIteration exception. This method corresponds to the tp_iternext slot of the type structure for Python objects in the Python/C API.

Therefore here is the good way to test if an element is an iterator:

    try:
        return it is iter(it)
    except:
        return False

Note: it is not sufficient to check for the existence of the methods since some sage object implement __iter__ to raise a NotImplemented exception !

Florent

CC: @sagetrac-sage-combinat @williamstein

Component: misc

Keywords: iterators, itertools

Author: Florent Hivert

Reviewer: Nicolas M. Thiéry

Merged: sage-4.2.1.rc0

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

@hivert

This comment has been minimized.

@hivert hivert changed the title Improved mantra to find whether an object is iterable (and get an iterator out it) Added is_iterator method and fixes sage to use it. Nov 5, 2009
@hivert
Copy link

hivert commented Nov 5, 2009

comment:2

Attachment: trac_7398_iter_detect-fh.patch.gz

@hivert hivert added this to the sage-4.2.1 milestone Nov 6, 2009
@nthiery
Copy link
Contributor Author

nthiery commented Nov 6, 2009

Attachment: trac_7398_iter_detect-fh.2.patch.gz

Updated patch after review by Nicolas (uses itertools to simplify further sage.server.interact.list_of_first_n)

@nthiery
Copy link
Contributor Author

nthiery commented Nov 6, 2009

comment:4

William: this makes a small edit to sage.server.notebook.interact.list_of_first_n

You may want to check/backport to the notebook code

@nthiery

This comment has been minimized.

@nthiery
Copy link
Contributor Author

nthiery commented Nov 6, 2009

Changed keywords from iterators to iterators, itertools

@hivert
Copy link

hivert commented Nov 6, 2009

comment:5

The given patch actually breaks somme code... I'm uploading a new one.

@hivert
Copy link

hivert commented Nov 6, 2009

comment:6

Attachment: trac_7398_iter_detect-fh.3.patch.gz

Nicolas : can you re-review this patch...
I commented out some line saying that it was never user and actually is was quite a lot... I don't understand why we didn't caught it by the tests. Anyway, Massena did.

Sorry for the mess,

Florent

@hivert
Copy link

hivert commented Nov 9, 2009

Attachment: trac_7398_combclass_set_compat-fh.patch.gz

@hivert
Copy link

hivert commented Nov 9, 2009

comment:7

The patch trac_7398_iter_detect-fh.3.patch broke something in graph_generators.
the patch trac_7398_combclass_set_compat-fh.patch should fix it.

Apply those two patches in that order.

Cheers,

Florent

@mwhansen
Copy link
Contributor

Merged: sage-4.2.1.rc0

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

3 participants