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

ENH: Enforce that sync-ness matches. #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions interface/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,24 @@ def _is_wrapper(f):
memo.add(id_func)
return func

def is_coroutine(f):
return False


else: # pragma: nocover-py2
from inspect import signature, Parameter, unwrap

try:
from inspect import CO_COROUTINE, CO_ITERABLE_COROUTINE
except ImportError:
# If we don't have these flags, there aren't any coroutines yet in
# Python 3.
def is_coroutine(f):
return False
else:
def is_coroutine(f):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using inspect.iscoroutinefunction instead ?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iscoroutinefunction only checks if a function was defined as an async def by looking at the CO_COROUTINE flag on the code object. For older codebases with lots of yield from coroutines, there's also the types.coroutine decorator, which sets a different flag, CO_ITERABLE_COROUTINE. This implementation handles both styles (and treats them as equivalent).

Copy link
Owner Author

@ssanderson ssanderson May 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, from reading the docs for iscoroutinefunction, I think it's just a bug that it doesn't handle types.coroutine functions correctly...

In [1]: from inspect import iscoroutinefunction

In [2]: iscoroutinefunction??
Signature: iscoroutinefunction(object)
Source:
def iscoroutinefunction(object):
    """Return true if the object is a coroutine function.

    Coroutine functions are defined with "async def" syntax,
    or generators decorated with "types.coroutine".
    """
    return bool((isfunction(object) or ismethod(object)) and
                object.__code__.co_flags & CO_COROUTINE)
File:      /usr/lib/python3.5/inspect.py
Type:      function

In [3]: import types

In [4]: @types.coroutine
   ...: def foo():
   ...:     yield 3
   ...:

In [5]: iscoroutinefunction(foo)
Out[5]: False

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it seems a bit more messy than what I originally thought:

Python 3.5.2 (default, Nov 23 2017, 16:37:01) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio, types, inspect
>>> asyncio.coroutine is types.coroutine
False
>>> @asyncio.coroutine
... def asyncio_coroutine():
...   return 'foo'
... 
>>> asyncio_coroutine()
<generator object asyncio_coroutine at 0x7f532573cdb0>
>>> inspect.iscoroutinefunction(asyncio_coroutine)
False
>>> inspect.iscoroutine(asyncio_coroutine())
False
>>> @types.coroutine
... def types_coroutine():
...   return 'foo'
... 
>>> types_coroutine()
'foo'
>>> inspect.iscoroutinefunction(types_coroutine)
False
>>> inspect.iscoroutine(types_coroutine())
False
>>> async def async_func():
...   return 'foo'
... 
>>> async_func()
<coroutine object async_func at 0x7f532573cdb0>
>>> inspect.iscoroutinefunction(async_func)
True
>>> inspect.iscoroutine(async_func())
True

So basically:

  • using async def foo() works as expected
  • using asyncio.coroutine generate a coroutine but inspect module doesn't realize this... I guess it's a bug
  • using types.coroutine doesn't even turn the function into a coroutine function (i.e. calling types_coroutine in the example returns the result instead of a coroutine), this is definitely a bug !

Note that I've done this test with CPython 3.5.2 and 3.6.3, both giving the same result

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've realized I made a mistake with types.coroutine which must take a generator function (I was testing with a regular function).
So my last bullet point was wrong, but anyway inspect doesn't work with it:

>>> import asyncio, types, inspect
>>> @types.coroutine
... def types_coroutine():
...   yield 42
... 
>>> types_coroutine()
<generator object types_coroutine at 0x7f36577fff68>
>>> inspect.iscoroutinefunction(types_coroutine)
False
>>> inspect.iscoroutine(types_coroutine())
False

return f.__code__.co_flags & (CO_COROUTINE | CO_ITERABLE_COROUTINE)

wraps = functools.wraps

exec("def raise_from(e, from_):" # pragma: nocover
Expand Down Expand Up @@ -96,3 +110,17 @@ class metaclass(meta):
def __new__(cls, name, this_bases, d):
return meta(name, bases, d)
return type.__new__(metaclass, 'temporary_class', (), {})


__all__ = [
'PY2',
'PY3',
'Parameter',
'is_coroutine',
'signature',
'unwrap',
'version_info',
'viewkeys',
'with_metaclass',
'wraps',
]
6 changes: 4 additions & 2 deletions interface/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def _diff_signatures(self, type_):
if not issubclass(impl_sig.type, iface_sig.type):
mistyped[name] = impl_sig.type

if not compatible(impl_sig.signature, iface_sig.signature):
if not compatible(impl_sig, iface_sig):
mismatched[name] = impl_sig

return missing, mistyped, mismatched
Expand Down Expand Up @@ -249,10 +249,12 @@ def _format_mismatched_types(self, mistyped):

def _format_mismatched_methods(self, mismatched):
return "\n".join(sorted([
" - {name}{actual} != {name}{expected}".format(
" - {actual_async}{name}{actual} != {expected_async}{name}{expected}".format(
name=name,
actual=bad_sig,
actual_async="async " if bad_sig.is_coroutine else "",
expected=self._signatures[name],
expected_async="async " if self._signatures[name].is_coroutine else "",
)
for name, bad_sig in mismatched.items()
]))
Expand Down
55 changes: 55 additions & 0 deletions interface/tests/_py3_interface_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
from textwrap import dedent

import pytest

from ..interface import implements, Interface, InvalidImplementation


def test_async_interface_sync_impl():

class AsyncInterface(Interface):
async def foo(self, a, b): # pragma: nocover
pass

# This should pass.
class AsyncImpl(implements(AsyncInterface)):
async def foo(self, a, b): # pragma: nocover
pass

# This should barf because foo isn't async.
with pytest.raises(InvalidImplementation) as e:
class SyncImpl(implements(AsyncInterface)):
def foo(self, a, b): # pragma: nocover
pass

actual_message = str(e.value)
expected_message = dedent(
"""
class SyncImpl failed to implement interface AsyncInterface:

The following methods of AsyncInterface were implemented with invalid signatures:
- foo(self, a, b) != async foo(self, a, b)"""
)
assert actual_message == expected_message


def test_sync_interface_async_impl():

class SyncInterface(Interface):
def foo(self, a, b): # pragma: nocover
pass

with pytest.raises(InvalidImplementation) as e:
class AsyncImpl(implements(SyncInterface)):
async def foo(self, a, b): # pragma: nocover
pass

actual_message = str(e.value)
expected_message = dedent(
"""
class AsyncImpl failed to implement interface SyncInterface:

The following methods of SyncInterface were implemented with invalid signatures:
- async foo(self, a, b) != foo(self, a, b)"""
)
assert actual_message == expected_message
48 changes: 39 additions & 9 deletions interface/tests/_py3_typecheck_tests.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
from inspect import signature
import types

from ..typecheck import compatible
from ..typed_signature import TypedSignature


def test_allow_new_params_with_defaults_with_kwonly():

@signature
@TypedSignature
def iface(a, b, c): # pragma: nocover
pass

@signature
@TypedSignature
def impl(a, b, c, d=3, e=5, *, f=5): # pragma: nocover
pass

Expand All @@ -19,11 +20,11 @@ def impl(a, b, c, d=3, e=5, *, f=5): # pragma: nocover

def test_allow_reorder_kwonlys():

@signature
@TypedSignature
def foo(a, b, c, *, d, e, f): # pragma: nocover
pass

@signature
@TypedSignature
def bar(a, b, c, *, f, d, e): # pragma: nocover
pass

Expand All @@ -33,11 +34,11 @@ def bar(a, b, c, *, f, d, e): # pragma: nocover

def test_allow_default_changes():

@signature
@TypedSignature
def foo(a, b, c=3, *, d=1, e, f): # pragma: nocover
pass

@signature
@TypedSignature
def bar(a, b, c=5, *, f, e, d=12): # pragma: nocover
pass

Expand All @@ -47,13 +48,42 @@ def bar(a, b, c=5, *, f, e, d=12): # pragma: nocover

def test_disallow_kwonly_to_positional():

@signature
@TypedSignature
def foo(a, *, b): # pragma: nocover
pass

@signature
@TypedSignature
def bar(a, b): # pragma: nocover
pass

assert not compatible(foo, bar)
assert not compatible(bar, foo)


def test_async_def_functions_are_coroutines():

@TypedSignature
async def foo(a, b): # pragma: nocover
pass

@TypedSignature
def bar(a, b): # pragma: nocover
pass

assert foo.is_coroutine
assert not compatible(foo, bar)


def test_types_dot_coroutines_are_coroutines():

@TypedSignature
@types.coroutine
def foo(a, b): # pragma: nocover
yield from foo()

@TypedSignature
def bar(a, b): # pragma: nocover
pass

assert foo.is_coroutine
assert not compatible(foo, bar)
4 changes: 4 additions & 0 deletions interface/tests/test_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -874,3 +874,7 @@ class BadImpl failed to implement interface HasMagicMethodsInterface:
- __getitem__(self, key)"""
)
assert actual_message == expected_message


if PY3:
from ._py3_interface_tests import * # noqa
39 changes: 27 additions & 12 deletions interface/tests/test_typecheck.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
from ..compat import PY3, signature
from ..compat import PY3
from ..typecheck import compatible
from ..typed_signature import TypedSignature


def test_compatible_when_equal():

@signature
@TypedSignature
def foo(a, b, c): # pragma: nocover
pass

assert compatible(foo, foo)

@signature
@TypedSignature
def bar(): # pragma: nocover
pass

Expand All @@ -20,11 +20,11 @@ def bar(): # pragma: nocover

def test_disallow_new_or_missing_positionals():

@signature
@TypedSignature
def foo(a, b): # pragma: nocover
pass

@signature
@TypedSignature
def bar(a): # pragma: nocover
pass

Expand All @@ -34,11 +34,11 @@ def bar(a): # pragma: nocover

def test_disallow_remove_defaults():

@signature
@TypedSignature
def iface(a, b=3): # pragma: nocover
pass

@signature
@TypedSignature
def impl(a, b): # pragma: nocover
pass

Expand All @@ -47,11 +47,11 @@ def impl(a, b): # pragma: nocover

def test_disallow_reorder_positionals():

@signature
@TypedSignature
def foo(a, b): # pragma: nocover
pass

@signature
@TypedSignature
def bar(b, a): # pragma: nocover
pass

Expand All @@ -61,11 +61,11 @@ def bar(b, a): # pragma: nocover

def test_allow_new_params_with_defaults_no_kwonly():

@signature
@TypedSignature
def iface(a, b, c): # pragma: nocover
pass

@signature
@TypedSignature
def impl(a, b, c, d=3, e=5, f=5): # pragma: nocover
pass

Expand All @@ -78,5 +78,20 @@ def test_first_argument_name():
assert TypedSignature(lambda: 0).first_argument_name is None


def test_regular_functions_arent_coroutines():

@TypedSignature
def foo(a, b, c): # pragma: nocover
pass

assert not foo.is_coroutine

@TypedSignature
def bar(a, b, c): # pragma: nocover
yield 1

assert not bar.is_coroutine


if PY3: # pragma: nocover
from ._py3_typecheck_tests import *
from ._py3_typecheck_tests import * # noqa
15 changes: 9 additions & 6 deletions interface/typecheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ def compatible(impl_sig, iface_sig):

Parameters
----------
impl_sig : inspect.Signature
impl_sig : interface.typed_signature.TypedSignature
The signature of the implementation function.
iface_sig : inspect.Signature
iface_sig : interface.typed_signature.TypedSignature
The signature of the interface function.

In general, an implementation is compatible with an interface if any valid
Expand All @@ -37,14 +37,17 @@ def compatible(impl_sig, iface_sig):
b. The return type of an implementation may be annotated with a
**subclass** of the type specified by the interface.
"""
real_impl_sig = impl_sig.signature
real_iface_sig = iface_sig.signature
return all([
impl_sig.is_coroutine == iface_sig.is_coroutine,
positionals_compatible(
takewhile(is_positional, impl_sig.parameters.values()),
takewhile(is_positional, iface_sig.parameters.values()),
takewhile(is_positional, real_impl_sig.parameters.values()),
takewhile(is_positional, real_iface_sig.parameters.values()),
),
keywords_compatible(
valfilter(complement(is_positional), impl_sig.parameters),
valfilter(complement(is_positional), iface_sig.parameters),
valfilter(complement(is_positional), real_impl_sig.parameters),
valfilter(complement(is_positional), real_iface_sig.parameters),
),
])

Expand Down
10 changes: 8 additions & 2 deletions interface/typed_signature.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"""
import types

from .compat import signature, unwrap
from .compat import is_coroutine, signature, unwrap
from .default import default


Expand All @@ -24,12 +24,18 @@ def __init__(self, obj):
if self._type is default:
self._type = type(obj.implementation)

self._signature = signature(extract_func(obj))
func = extract_func(obj)
self._signature = signature(func)
self._is_coroutine = is_coroutine(func)

@property
def signature(self):
return self._signature

@property
def is_coroutine(self):
return self._is_coroutine

@property
def first_argument_name(self):
try:
Expand Down