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

Add more common interfaces #175

Merged
merged 8 commits into from Feb 21, 2020
Merged

Add more common interfaces #175

merged 8 commits into from Feb 21, 2020

Conversation

jamadden
Copy link
Member

@jamadden jamadden commented Feb 10, 2020

Derived automatically from Python stdlib ABCs.

When done, this will fix #138.

So far, its just the collection classes. Remaining items:

  • Whoops, I missed ByteString
  • The numbers ABCs
  • Other common non-ABC types, like IText and whatever else is missing from dolmen.builtins
  • What happens to the "legacy" interfaces in mapping.py and sequence.py? Are they left completely alone? They are probably badly out of date. Do we docs-deprecate them? Maybe they can slot-in as super-classes

@jamadden jamadden marked this pull request as ready for review February 13, 2020 17:27
@jamadden jamadden changed the title [WIP] Add more common interfaces Add more common interfaces Feb 13, 2020
@jamadden
Copy link
Member Author

jamadden commented Feb 13, 2020

This still leaves classes in control of what interfaces they implement, unlike the virtual duck-typed subclassing of ABCs, as described in #135

It gets us most of the way there, though. If we want to do it, it's now pretty easy with a small change to the definition of implementedBy (and maybe a corresponding tweak to ABCInterface.providedBy for direct queries; not shown here):

-------------------------------------------------------------------------------------------------
modified: src/zope/interface/declarations.py
-------------------------------------------------------------------------------------------------
@@ -275,6 +275,27 @@ def _implements_name(ob):
     return (getattr(ob, '__module__', '?') or '?') + \
         '.' + (getattr(ob, '__name__', '?') or '?')

+def _abc_interfaces():
+    try:
+        from zope.interface.common import ABCInterface, ABCInterfaceClass
+    except ImportError:
+        return [] # Circular imports at _wire() time.
+    from zope.interface.common import collections, numbers, io # Ensure defined
+
+    seen = set()
+    to_examine = list(ABCInterface.dependents)
+    while to_examine:
+        iface = to_examine.pop()
+        if iface in seen:
+            continue
+        seen.add(iface)
+        to_examine.extend(iface.dependents)
+    return [x for x in seen if isinstance(x, ABCInterfaceClass)]
+
+def _abc_interfaces_of(cls):
+    ifaces = _abc_interfaces()
+
+    return [iface for iface in ifaces if issubclass(cls, iface.getABC())]

 @_use_c_impl
 def implementedBy(cls):
@@ -338,7 +359,8 @@ def implementedBy(cls):
                 raise TypeError("ImplementedBy called for non-factory", cls)
             bases = ()

-        spec = Implements.named(spec_name, *[implementedBy(c) for c in bases])
+        spec = Implements.named(spec_name,
+                                *([implementedBy(c) for c in bases] + _abc_interfaces_of(cls)))
         spec.inherit = cls

     try:

With that change, classes that "subclass" ABCs automatically also provide those interfaces:

>>> from zope.interface.common import collections
>>> class Sized(object):
   ...:     def __len__(self): return 0
   ...:
>>> from zope.interface import providedBy
>>> list(providedBy(Sized()))
[<ABCInterfaceClass zope.interface.common.collections.IHashable>,
 <ABCInterfaceClass zope.interface.common.collections.ISized>]

That's a rough approximation, of course. The biggest issue is later registrations and cache invalidations. E.g., this doesn't do what you'd expect:

>>> class VirtualSized(object):
...     pass
...
>>> list(providedBy(VirtualSized()))
[<ABCInterfaceClass zope.interface.common.collections.IHashable>]
>>> from collections.abc import Sized
>>> Sized.register(VirtualSized)
<class '__main__.VirtualSized'>
>>> isinstance(VirtualSized(), Sized)
True
>>> list(providedBy(VirtualSized()))
[<ABCInterfaceClass zope.interface.common.collections.IHashable>]

I feel like that's solvable, though.

Is that something we want to proceed with? I'm not sure I personally have a defined use-case for it right now, but OTOH, I think I remember writing things like classImplements(OOBTree, IMapping); classImplements(dict, IMapping) just to be able to use the same adapters for both OOBTree and dict, and this would make that much more convenient (except for some reason, OOBTree isn't actually a virtual subclass of collections.abc.Mapping 😞 zopefoundation/BTrees#121).

@jensens
Copy link
Member

jensens commented Feb 17, 2020

Overall this looks fine. I must admit, I can not judge every detail.

@jamadden
Copy link
Member Author

Thanks for the feedback!

Copy link
Member

@ale-rt ale-rt left a comment

Choose a reason for hiding this comment

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

LGTM, but maybe wait for @mgedmin review or some other experienced dev!

@mgedmin
Copy link
Member

mgedmin commented Feb 21, 2020

This is rather a big diff and I'm not feeling up to reviewing it right now.

(Lack of review from little old me should not be a blocker to merging!)

@jamadden
Copy link
Member Author

This is rather a big diff and I'm not feeling up to reviewing it right now.

No worries! Thank you.

I will go ahead and merge as I'd like to make a release soon. Since it's completely opt-in I don't think it can do any harm.

@jamadden jamadden merged commit c931999 into master Feb 21, 2020
@jamadden jamadden deleted the issue138 branch February 21, 2020 13:12
@jamadden jamadden added this to the 5.0 milestone Mar 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can we hook zope.interface.common up to standard library ABCs/classes?
4 participants