classification
Title: Make isinstance() work with super type instances
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, benjamin.peterson, brett.cannon, pitrou, rhettinger
Priority: low Keywords:

Created on 2013-06-21 01:33 by brett.cannon, last changed 2020-03-06 20:21 by brett.cannon. This issue is now closed.

Files
File name Uploaded Description Edit
simple_diamond_example.py rhettinger, 2013-06-21 22:22 Simple diamond example
Messages (12)
msg191550 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 01:33
class A: pass
class B(A): pass

sup = super(B, B())
isinstance(sup, A)  # returns False

Why is that? Is there an explicit reason to prevent that isinstance check from working? How about just for ABCs? Either way it's annoying that at least for ABCs you can't check against a super object.
msg191559 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-21 08:52
Does anyone actually check for instance-ness of super objects? I would never have thought of such an idiom.
msg191581 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 13:55
So I have a use case for this actually. =)

http://bugs.python.org/issue17621 is about trying to finally add a lazy mixin to importlib so people stop asking for it. I have a version already written externally at https://code.google.com/p/importers/source/browse/importers/lazy.py. Part of the trick of making it work is having a way to drop the lazy mixin from the loader MRO for all future uses (which is important for reloads) is to assign __loader__ to super(Mixin, self). The problem is that if you use isinstance on that loader with any of the ABCs in importlib.abc it will always come back false no matter whether the interface is actually implemented or not.

While I consider it a marginal thing for people to do (importlib.abc is for making sure people implement the right methods and providing default implementations, not for interface checking), I'm sure someone will do it and I would like one less possible warning in any future docs about this lazy mixin regarding isinstance checks.
msg191582 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-21 14:02
> http://bugs.python.org/issue17621 is about trying to finally add a
> lazy mixin to importlib so people stop asking for it. I have a
> version already written externally at
> https://code.google.com/p/importers/source/browse/importers/lazy.py.
> Part of the trick of making it work is having a way to drop the lazy
> mixin from the loader MRO for all future uses (which is important
> for reloads) is to assign __loader__ to super(Mixin, self).

That sounds like a better job for a proxy rather than a mixin.
msg191590 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 15:01
On Fri, Jun 21, 2013 at 10:02 AM, Antoine Pitrou <report@bugs.python.org>wrote:

>
> Antoine Pitrou added the comment:
>
> > http://bugs.python.org/issue17621 is about trying to finally add a
> > lazy mixin to importlib so people stop asking for it. I have a
> > version already written externally at
> > https://code.google.com/p/importers/source/browse/importers/lazy.py.
> > Part of the trick of making it work is having a way to drop the lazy
> > mixin from the loader MRO for all future uses (which is important
> > for reloads) is to assign __loader__ to super(Mixin, self).
>
> That sounds like a better job for a proxy rather than a mixin.
>

I'm attracted to the idea of making a loader lazy by simply doing something
like ``class LazySourceFileLoader(LazyMixin, SourceFileLoader): ...``. The
key thing is that it doesn't require proxying the constructor in situations
like PathEntryFinder where the finder is going to be making your loader
instance (or at least calling the object). I mean you could override
__call__ to store the arguments and then simply use them when needed to
construct the real loader, but I don't know if that makes the code simpler
without actually coding it up.
msg191592 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-21 15:11
> I'm attracted to the idea of making a loader lazy by simply doing
> something
> like ``class LazySourceFileLoader(LazyMixin, SourceFileLoader):
> ...``.

But then you have to make a specific Lazy subclass for each delegated
loader implementation. With a proxy class you would simply proxy each
loader instance:

existing_loader = SourceFileLoader(...)
lazy_loader = LazyLoader(existing_loader)
...
msg191602 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 17:51
On Fri, Jun 21, 2013 at 11:11 AM, Antoine Pitrou <report@bugs.python.org>wrote:

>
> Antoine Pitrou added the comment:
>
> > I'm attracted to the idea of making a loader lazy by simply doing
> > something
> > like ``class LazySourceFileLoader(LazyMixin, SourceFileLoader):
> > ...``.
>
> But then you have to make a specific Lazy subclass for each delegated
> loader implementation. With a proxy class you would simply proxy each
> loader instance:
>
> existing_loader = SourceFileLoader(...)
> lazy_loader = LazyLoader(existing_loader)
> ...

But the point I tried to make earlier is that approach doesn't work
directly when you are creating the loader instances dynamically within a
finder. E.g. FileFinder (
http://docs.python.org/3/library/importlib.html#importlib.machinery.FileFinder)
instantiates the loader for you.

Now you could tweak things to make this work, but it isn't quite as
straightforward as you suggest because of this need to have a callable for
finders to use to create new loaders:

class LazyProxy(importlib.abc.Loader):

  def __init__(self, loader):
    self.loader = loader

  def __call__(self, *args, **kwargs):
    self.args = args
    self.kwargs = kwargs
    return self

  def load_module(self, fullname):
    # XXX ignoring sys.modules details, e.g. if module already existed.
    lazy_module = LazyModule(fullname, proxy=self, name=fullname)
    sys.modules[fullname] = lazy_module
    return lazy_module

class LazyModule(types.ModuleType):

    def __init__(*args, proxy, name, **kwargs):
      self.__proxy = proxy
      self.__name = name
      super().__init__(*args, **kwargs)

    def __getattribute__(self, attr):
      self.__class__ = Module
      state = self.__dict__.copy()
      loader = self.__proxy.loader(*self.proxy.args, **self.proxy.kwargs)
      # XXX ignoring sys.modules details, e.g. removing module on load
failure.
      loader.load_module(self.__name)
      self.__dict__.update(state)
      return getattr(module, attr)

That's all totally untested (not even verified to compile) but I think that
would do what you are suggesting.
msg191605 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-06-21 19:22
I'm not sure this is a theoretically sound idea.  Do you know what CLOS does in the same circumstance?
msg191607 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 19:57
I have no idea what CLOS does in this situation.

I'm fine with this never being changed, but I wanted to at least file the bug to have the discussion in case it ever comes up again.
msg191615 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2013-06-21 22:22
> I have no idea what CLOS does in this situation.
No worries, I was just curious whether you knew whether this was a solved problem in Dylan or CLOS or some other language.

When faced with a diamond diagram, what are the semantics for isinstance(super_instance, cls)?

I worked on it for a little bit and came up with the following:

def super_isinstance(super_inst, cls):
    'Is the cls in the mro somewhere after the current class?'
    mro = super_inst.__self__.__class__.__mro__
    thisclass = super_inst.__thisclass__
    return cls in mro and mro.index(thisclass) < mro.index(cls)
msg191619 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-06-21 22:40
That solution looks right. Problem is that it would need to either go directly into isinstance() or type would need to grow an __instancecheck__ to deal with this case since you can't override isinstance from the instance end. That I'm not sure people are okay with.
msg363542 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2020-03-06 20:21
After 6 years and no really movement I don't think we are going to bother to change the semantics for this. :)
History
Date User Action Args
2020-03-06 20:21:57brett.cannonsetstatus: open -> closed
resolution: wont fix
messages: + msg363542

stage: test needed -> resolved
2014-02-06 19:02:54brett.cannonunlinkissue17621 dependencies
2013-06-22 09:44:49Arfreversetnosy: + Arfrever
2013-06-21 22:40:13brett.cannonsetmessages: + msg191619
2013-06-21 22:35:08brett.cannonlinkissue17621 dependencies
2013-06-21 22:22:49rhettingersetfiles: + simple_diamond_example.py

messages: + msg191615
2013-06-21 19:57:06brett.cannonsetmessages: + msg191607
2013-06-21 19:22:20rhettingersetnosy: + rhettinger
messages: + msg191605
2013-06-21 17:51:13brett.cannonsetmessages: + msg191602
2013-06-21 15:11:22pitrousetmessages: + msg191592
2013-06-21 15:01:38brett.cannonsetmessages: + msg191590
2013-06-21 14:02:22pitrousetmessages: + msg191582
2013-06-21 13:55:49brett.cannonsetpriority: normal -> low

messages: + msg191581
2013-06-21 08:52:45pitrousetnosy: + pitrou, benjamin.peterson
messages: + msg191559
2013-06-21 01:33:23brett.cannoncreate