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

Contains/Not Contains throws wrong errors #877

Merged
merged 10 commits into from Aug 9, 2018

Conversation

Projects
None yet
2 participants
@patiences
Contributor

patiences commented Jul 27, 2018

  1. in/__contains__ throws the wrong error on non-iterable types (like int) -- should be TypeError not AttributeError
  2. not in/__not_contains__ should probably use __contains__ instead of looking for __not_contains__ attribute
  3. org.python.types.Str.contains should return a org.python.types.Bool

This PR makes the following changes:

  1. Addresses the problems above
  2. Removes the __not_contains__ attribute from org/python/Objects. I verified that each __not_contains__ attribute that was defined was just an inverse of its __contains__ attribute. Since the operator not in is defined to have the inverse true value of in (from https://docs.python.org/3/reference/expressions.html#membership-test-operations), there is no need to define separate __not_contains__ attributes, they should all direct to __contains__. See org/python/types/Object.java#not_contains.

@patiences patiences referenced this pull request Jul 27, 2018

Merged

Comparisons optimization #875

patiences added some commits Jul 27, 2018

@patiences patiences changed the title from Contains/Not Contains throws wrong errors to [WIP] Contains/Not Contains throws wrong errors Jul 27, 2018

@patiences patiences changed the title from [WIP] Contains/Not Contains throws wrong errors to Contains/Not Contains throws wrong errors Jul 30, 2018

@@ -552,14 +552,6 @@ public void __delitem__(org.python.Object index) {
throw new org.python.exceptions.AttributeError(this, "__contains__");
}

This comment has been minimized.

@patiences

patiences Jul 30, 2018

Contributor

I'm not sure that this AttributeError is strictly necessary -- seems like a __contains__ call is either successful (it's defined elsewhere) or a TypeError is thrown (and this is constructed after this AttributeError is thrown and caught). Could this be replaced by org.python.types.NotImplemented?

@freakboy3742

👍

@freakboy3742 freakboy3742 merged commit 2262a71 into pybee:master Aug 9, 2018

5 checks passed

beekeeper:0/beefore:javacheckstyle Java lint checks passed.
Details
beekeeper:0/beefore:pycodestyle Python lint checks passed.
Details
beekeeper:1/smoke-test Smoke build (Python 3.4) passed.
Details
beekeeper:2/full-test:py3.5 Python 3.5 tests passed.
Details
beekeeper:2/full-test:py3.6 Python 3.6 tests passed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment