-
Notifications
You must be signed in to change notification settings - Fork 70
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
Fix verification for methods of builtin types with pseudo-default args on PyPy #172
Conversation
…uments on Pypy On PyPy2, they are ignored (like on CPython), but on PyPy3 they can actually be validated. Fixes #118
OT: While reading this changes the question comes up when we drop Python 2 support. zope.interface 6? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I know large portions of the user base are still using Python 2. I will maintain Python 2 support for the foreseeable future. |
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
require methods that were removed from Python 3 on Python 3, such as | ||
``__setslice___``. Now, ``dict``, ``list`` and ``tuple`` properly | ||
verify as ``IFullMapping``, ``ISequence`` and ``IReadSequence,`` | ||
respectively on all versions of Python. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+:100:
return verifyClass(iface, klass) | ||
return verifyClass | ||
|
||
_adjust_object_before_verify = lambda self, x: x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be more readable as a regular def
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's an idiom I've fallen into. I use it primarily in tests as a signal that it will definitely be overridden by subclasses, possibly at runtime. I think it began in abstract base classes as a way to avoid introducing uncovered lines.
def _callFUT(self, iface, klass, **kwargs): | ||
return self.verifier(iface, | ||
self._adjust_object_before_verify(klass), | ||
**kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, this is funky sleight of hand I don't understand, but maybe things will become clearer if I read more of the diff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(no I did not understand why all this magic was necessary)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which part seems magic?
The idea here was that rather than redefining the entirety of _callFUT
in the subclass (I tried a few variations on that, all of them were repetitive), the subclass just defines adjust_object_before_verify
.
That's fine. It's not much work here. |
On PyPy2, they are ignored (like on CPython), but on PyPy3 they can actually be validated.
Fixes #118
To add tests for this, some minor changes to the common interfaces that remove missing methods on Python 3 were needed.