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

useless __list__ methods #2626

Closed
sagetrac-cwitty mannequin opened this issue Mar 21, 2008 · 6 comments
Closed

useless __list__ methods #2626

sagetrac-cwitty mannequin opened this issue Mar 21, 2008 · 6 comments
Assignees
Milestone

Comments

@sagetrac-cwitty
Copy link
Mannequin

sagetrac-cwitty mannequin commented Mar 21, 2008

Sage 2.10.4 contains 5 __list__ methods, which are never called. Apparently the authors of these methods thought that list(x) would call x.__list__(), but it does not; the Python source code contains no instance of the string "__list__".

list(x) does call x.__iter__(), which is how the doctests on these methods manage to work.

The methods should be removed, so as not to mislead future developers into thinking they do something.

sage: search_src('__list__')
----------------------------------------------------------------------
| SAGE Version 2.10.4, Release Date: 2008-03-17                      |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
crypto/mq/mpolynomialsystem.py:    def __list__(self):
crypto/mq/mpolynomialsystem.py:    def __list__(self):
crypto/mq/sbox.py:    def __list__(self):
schemes/elliptic_curves/ell_point.py:    def __list__(self):
schemes/generic/morphism.py:    def __list__(self):

Component: misc

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

@sagetrac-cwitty sagetrac-cwitty mannequin added this to the sage-2.11 milestone Mar 21, 2008
@sagetrac-cwitty sagetrac-cwitty mannequin self-assigned this Mar 21, 2008
@malb
Copy link
Member

malb commented Mar 21, 2008

fixes the issue for crypto.mq

@malb
Copy link
Member

malb commented Mar 21, 2008

Attachment: crypto_mq__list__.patch.gz

fixes the issue for elliptic curve points over fields

@malb
Copy link
Member

malb commented Mar 21, 2008

Attachment: ellpoint__list__.patch.gz

Attachment: morphism__list__.patch.gz

@malb
Copy link
Member

malb commented Mar 21, 2008

comment:1

The attached patches remove all __list__ instances mentioned above. I don't know how I got the idea that there is a special method called __list__ in the first place.

PS: I refuse to write a doctest for SchemeMorphism_coordinates because I don't even know how to construct such a thing! This class doesn't have a single line of documentation (though it is short to be fair).

@malb malb changed the title useless __list__ methods [need review] useless __list__ methods Mar 21, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 22, 2008

comment:2

Nice patches, all the doctests pass [with known exception]. malb is correct in skipping doctests for SchemeMorphism_coordinates - hopefully somebody else will take care of that in a subsequent patch. Positive review.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title [need review] useless __list__ methods useless __list__ methods Mar 22, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 22, 2008

comment:3

Merged in Sage 2.11.alpha1

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Mar 22, 2008
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

1 participant