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

GapElement.__contains__ (from libgap) should use the infix in from gap #23844

Closed
videlec opened this issue Sep 13, 2017 · 10 comments
Closed

GapElement.__contains__ (from libgap) should use the infix in from gap #23844

videlec opened this issue Sep 13, 2017 · 10 comments

Comments

@videlec
Copy link
Contributor

videlec commented Sep 13, 2017

Right now __contains__ is not defined and defaults to the Python behavior, which does some very wrong stuff when both sides are GapElement. The following gets right with the branch applied

sage: 1 in libgap(ZZ)
Traceback (most recent call last):
...
TypeError: argument of type 'sage.libs.gap.element.GapElement_Ring' is not iterable

Component: interfaces

Keywords: thursdaysbdx

Author: Vincent Delecroix

Branch/Commit: f8c0088

Reviewer: Travis Scrimshaw

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

@videlec videlec added this to the sage-8.1 milestone Sep 13, 2017
@videlec
Copy link
Contributor Author

videlec commented Sep 14, 2017

Changed keywords from none to thursdaysbdx

@videlec
Copy link
Contributor Author

videlec commented Oct 4, 2017

Branch: u/vdelecroix/23844

@videlec
Copy link
Contributor Author

videlec commented Oct 4, 2017

New commits:

f8c008823844: infix 'in' for GapElement

@videlec
Copy link
Contributor Author

videlec commented Oct 4, 2017

Commit: f8c0088

@videlec
Copy link
Contributor Author

videlec commented Oct 4, 2017

Author: Vincent Delecroix

@videlec

This comment has been minimized.

@tscrim

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Oct 9, 2017

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Oct 9, 2017

comment:4

LGTM (although Element does not define a __contains__, so I updated the description).

@vbraun
Copy link
Member

vbraun commented Oct 16, 2017

Changed branch from u/vdelecroix/23844 to f8c0088

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