Repository: seequent/pure_interface

Issue:
Implementation attribute check failing if exception raised within property
The following code snippet produces the error: 
TypeError: AnimalAttribute.__init__ does not create required attribute "height"
```python
class IAnimalAttribute(PureInterface):
    height = None


class AnimalAttribute(object, IAnimalAttribute):
    @property
    def height(self):
        raise Exception("Bang")


animal = AnimalAttribute()
```

This is caused by python 2 hasattr behaviour meaning if any exception in a property is raised hasattr returns false. Also confusingly the following works, contrary to the README which states "The use of class attribute or @property to define a class attribute are interchangeable":
```python 
class IAnimal(PureInterface):
    @abstractproperty
    def height(self):
        pass


class Animal(object, IAnimal):
    @property
    def height(self):
        raise Exception("Bang")


animal = Animal()
```

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index 82807e6..9ba6420 100644
--- a/README.rst
+++ b/README.rst
@@ -8,7 +8,8 @@ A Python interface library that disallows function body content on interfaces an
 
 Jump to the `Reference`_.
 
-**Features:**
+Features
+--------
     * Prevents code in method bodies of an interface class
     * Ensures that method overrides have compatible signatures
     * Allows concrete implementations the flexibility to implement abstract properties as instance attributes.
@@ -32,7 +33,7 @@ pure_interface depends on the six_ and typing_ modules (typing is included in py
 .. _six: https://pypi.python.org/pypi/six
 .. _typing: https://pypi.python.org/pypi/typing
 
-You can install released versions of pure_interface using pip::
+You can install released versions of ``pure_interface`` using pip::
 
     pip install pure_interface
 
@@ -64,16 +65,17 @@ on Python 2.7 ``abstractclassmethod`` and ``abstractstaticmethod`` are also avai
 
 However these decorators are optional as **ALL** methods and properties on a pure interface are abstract.  In the
 example above, both ``height`` and ``speak`` are considered abstract and must be overridden by subclasses.
-Because of this, interface classes cannot be instantiated ::
-
-    IAnimal()
-    TypeError: Interfaces cannot be instantiated.
 
 Including abstract decorators in your code can be useful for reminding yourself (and telling your IDE) that you need
 to override those methods.  Another common way of informing an IDE that a method needs to be overridden is for
 the method to raise ``NotImplementedError``.  For this reason methods that just raise ``NotImplementedError`` are also
 considered empty.
 
+Interface classes cannot be instantiated ::
+
+    IAnimal()
+    TypeError: Interfaces cannot be instantiated.
+
 Including code in a method will result in an ``InterfaceError`` being raised when the module is imported. For example::
 
     class BadInterface(PureInterface):
@@ -84,11 +86,12 @@ Including code in a method will result in an ``InterfaceError`` being raised whe
     Did you forget to inherit from object to make the class concrete?
 
 Inspired by PEP-544_ ``pure_interface`` also allows using class attributes to specify required interface attributes.
+This is now the preferred way to specify the existance of an attribute in an interface.
+The use of class attribute or ``@property`` to define a the existance of an attribute are usually interchangable [#attr]_.
 
 .. _PEP-544: https://www.python.org/dev/peps/pep-0544/
 
-The use of class attribute or ``@property`` to define a class attribute are interchangable. This interface is equivalent
-to the one above::
+This interface is equivalent to the one above::
 
     class IAnimal(PureInterface):
         height = None
@@ -104,7 +107,7 @@ The value assigned to class attributes *must* be ``None`` and the attribute is r
 This is because ``IAnimal`` is an interface definition and not an implementation.  Of course, concrete implementations
 may use class attributes as normal.
 
-In Python 3.6 and later type annotations can also be used to define interface properties::
+In Python 3.6 and later type annotations are the preferred way to define interface attributes::
 
     class IAnimal(PureInterface):
         height: float
@@ -150,6 +153,8 @@ and properties that satisfy the empty method criteria will result in a type that
         def bar(self):
             pass
 
+.. _attributes:
+
 Concrete implementations may implement interface properties as normal attributes,
 provided that they are all set in the constructor::
 
@@ -165,8 +170,8 @@ You can also implement interface class attributes as properties if desired.
 
 The astute reader will notice that the ``Animal2`` bases list makes an inconsistent method resolution order.
 This is handled by the ``PureInterfaceType`` meta-class by removing ``object`` from the front of the bases list.
-However static checkers such as mypy_ will complain.  To get around this, ``pure_interface`` includes an empty
-``Concrete`` class which you can use to keep mypy happy::
+However static checkers such as mypy_ and some IDE's will complain.  To get around this, ``pure_interface`` includes an empty
+``Concrete`` class which you can use to keep mypy and your IDE happy::
 
     class Concrete(object):
         pass
@@ -579,3 +584,31 @@ Module Attributes
 **missing_method_warnings**
     The list of warning messages for concrete classes with missing interface (abstract) method overrides.
     Note that missing properties are NOT checked for as they may be provided by instance attributes.
+
+Footnotes
+---------
+
+.. [#attr] For the sake of clarity lets consider 2 interfaces and 2 concrete implementations
+
+::
+
+    class I1(PureInterface)
+        @property
+        def foo(self):
+            pass
+
+    class I2(PureInterface):
+        foo = None
+
+    class C1(I1):
+        def __init__(self):
+            self.foo = 1
+
+    class C2(I2):
+        def __init__(self):
+            self.foo = 1
+
+
+When constructing class ``C1`` the meta-class creates a property for ``foo`` to override the abstract property on ``I1``.
+When constructing class ``C2`` this is not necessary.
+This also means that  ``C1().foo`` is a descriptor call whereas ``C2().foo`` is a faster ``__dict__`` lookup.
diff --git a/pure_interface.py b/pure_interface.py
index e3fb607..1791684 100644
--- a/pure_interface.py
+++ b/pure_interface.py
@@ -77,7 +77,8 @@ class _PIAttributes(object):
     def __init__(self, type_is_interface, interface_method_signatures, interface_property_names,
                  interface_attribute_names):
         self.type_is_pure_interface = type_is_interface
-        self.abstractproperties = frozenset()  # properties that must be provided by instances
+        # abstractproperties are checked for at instantiation
+        self.abstractproperties = frozenset(interface_attribute_names)
         self.interface_method_names = frozenset(interface_method_signatures.keys())  # type: FrozenSet[str]
         self.interface_property_names = frozenset(interface_property_names)  # type: FrozenSet[str]
         self.interface_attribute_names = frozenset(interface_attribute_names)  # type: FrozenSet[str]
@@ -269,7 +270,7 @@ def _get_instructions(code_obj):
 
 
 def _is_descriptor(obj):  # in our context we only care about __get__
-    return hasattr(obj, '__get__')
+    return hasattr(obj, '__get__') and not isinstance(obj, types.FunctionType)
 
 
 def _signature_info(arg_spec):
@@ -528,8 +529,6 @@ class PureInterfaceType(abc.ABCMeta):
                     missing_method_warnings.append(message)
                     warnings.warn(message, stacklevel=stacklevel)
 
-        if type_is_interface and not cls.__abstractmethods__:
-            cls.__abstractmethods__ = frozenset({''})  # empty interfaces still should not be instantiated
         return cls
 
     def __call__(cls, *args, **kwargs):
@@ -540,9 +539,6 @@ class PureInterfaceType(abc.ABCMeta):
         for attr in cls._pi.abstractproperties:
             if not hasattr(self, attr):
                 raise TypeError('{}.__init__ does not create required attribute "{}"'.format(cls.__name__, attr))
-        for attr in cls._pi.interface_attribute_names:
-            if not hasattr(self, attr):
-                raise TypeError('{}.__init__ does not create required attribute "{}"'.format(cls.__name__, attr))
         return self
 
     def __dir__(cls):


Test patch:
diff --git a/tests/test_implementation_checks.py b/tests/test_implementation_checks.py
index 46e87eb..cfc2dc6 100644
--- a/tests/test_implementation_checks.py
+++ b/tests/test_implementation_checks.py
@@ -63,6 +63,20 @@ class ISimple(pure_interface.PureInterface):
         pass
 
 
+class ICrossImplementation(pure_interface.PureInterface):
+    """ interface to test class attributes implemented as properties and vice versa """
+    a = None
+    b = None
+
+    @property
+    def c(self):
+        pass
+
+    @property
+    def d(self):
+        pass
+
+
 class TestImplementationChecks(unittest.TestCase):
     def test_instantiation_fails(self):
         with self.assertRaises(TypeError):
@@ -379,6 +393,12 @@ class IAttribute(pure_interface.PureInterface):
     a = None
 
 
+class RaisingProperty(object, IAttribute):
+    @property
+    def a(self):
+        raise Exception("Bang")
+
+
 class TestAttributeImplementations(unittest.TestCase):
     def test_class_attribute_in_interface(self):
         self.assertIn('a', pure_interface.get_interface_attribute_names(IAttribute))
@@ -443,12 +463,19 @@ class TestAttributeImplementations(unittest.TestCase):
             exec(py_36_tests)
 
     def test_mock_spec_includes_attrs(self):
-        m = mock.MagicMock(spec=IAttribute)
+        m = mock.MagicMock(spec=IAttribute, instance=True)
         try:
             x = m.a
         except AttributeError:
             self.fail("class attribute not mocked")
 
+    def test_raising_property(self):
+        """ Issue 23 """
+        try:
+            a = RaisingProperty()
+        except:
+            self.fail('Instantiation with property that raises failed')
+
 
 py_36_tests = """
 def test_annotations(self):
@@ -469,3 +496,25 @@ def test_annotations2(self):
 test_annotations(self)
 test_annotations2(self)
 """
+
+
+class TestCrossImplementations(unittest.TestCase):
+    """ test class attributes implemented as properties and vice versa """
+    def test_cross_implementations(self):
+        class CrossImplementation(pure_interface.Concrete, ICrossImplementation):
+            def __init__(self):
+                self.a = 1
+                self.c = 2
+
+            @property
+            def b(self):
+                return 3
+
+            @property
+            def d(self):
+                return 4
+
+        c = CrossImplementation()
+        self.assertEqual(frozenset(['a', 'c']), CrossImplementation._pi.abstractproperties)
+        self.assertEqual(frozenset(['a', 'b']), CrossImplementation._pi.interface_attribute_names)
+        self.assertEqual(frozenset(['c', 'd']), CrossImplementation._pi.interface_property_names)