[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Brandt Bucher]

Change by Brandt Bucher :


--
nosy: +brandtbucher

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

All typing related tests are fixed by updating 
types.GenericAlias.__getattribute__. So the only thing is left to make all 
tests passed is removing some special methods tests.

The lookup of __instancecheck__ and __subclasscheck__ was changed by Benjamin 
in fb6fb062e8f677dd63943f3a4b8a45c6665b3418. It looks like deliberate change. 
Benjamin, do you remember the reasons of the original change?

--
nosy: +benjamin.peterson

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Guido van Rossum]

Guido van Rossum  added the comment:

I believe that the PEP 3119 example doesn't work (I've confirmed something
simpler) but I have a feeling that the uses of __instancecheck__ in
typing.py are actually okay. For example, isinstance(42, typing.Any)
correctly calls _SpecialForm.__instancecheck__ -- because Any is not a
type, it's an instance of typing._SpecialForm so it all works out
correctly. (I haven't checked out the other -- @Serhiy do you have time to
look into that one?)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Here's an example in PEP 3119 that appears to work but actually bypasses the 
method:

from abc import ABCMeta, abstractmethod

class Sized(metaclass=ABCMeta):
@abstractmethod
def __hash__(self):
return 0
@classmethod
def __instancecheck__(cls, x):
print('Called')
return hasattr(x, "__len__")
@classmethod
def __subclasscheck__(cls, C):
return hasattr(C, "__bases__") and hasattr(C, "__len__")

$ python3.10 -i demo_3119.py
>>> isinstance([], Sized)
True

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Most of failed tests are related to typing, but there is a purposed test for 
__instancecheck__ and __subclasscheck__ bypassing __getattr__ and 
__getattribute__ 
(test.test_descr.ClassPropertiesAndMethods.test_special_method_lookup).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Raymond Hettinger]

Raymond Hettinger  added the comment:

I did a scan of the standard library and code in the wild.  It looks like 
almost all uses are in metaclasses (which makes sense because that matches the 
AppendableSequence example in PEP 3119).

However, the typing module does have some cases of __instancecheck__ being used 
in regular classes.  AFAICT that code is completely inoperative and there is no 
way for it to ever get called by isinstance().

I think people who __instancecheck__ in regular classes aren't noticing that 
their code isn't being called at all.  The likely reasons are that isinstance() 
has a fast path for exact type matches and type() defines an __instancecheck__ 
in a reasonable way.  So unless someone tests a case not already covered by 
those two paths, they won't notice the dead code.

The two cases in typing.py are _Final and _BaseGenericAlias.  The 
__instancecheck__ methods in those were written by Ivan and by GvR likely with 
the expectation that that code would by called by isinstance().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Failed tests:

==
FAIL: test_isinstance_with_or_union 
(test.test_isinstance.TestIsInstanceIsSubclass)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_isinstance.py", line 223, in 
test_isinstance_with_or_union
with self.assertRaises(TypeError):
^^
AssertionError: TypeError not raised

==
FAIL: test_isinstance (test.test_genericalias.BaseTest)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_genericalias.py", line 249, in 
test_isinstance
with self.assertRaises(TypeError):
^^
AssertionError: TypeError not raised

==
FAIL: test_issubclass (test.test_genericalias.BaseTest)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_genericalias.py", line 255, in 
test_issubclass
with self.assertRaises(TypeError):
^^
AssertionError: TypeError not raised

==
FAIL: test_callable_instance_type_error 
(test.test_typing.CollectionsCallableTests)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_typing.py", line 490, in 
test_callable_instance_type_error
with self.assertRaises(TypeError):
^^
AssertionError: TypeError not raised

==
FAIL: test_self_subclass (test.test_typing.CollectionsCallableTests)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_typing.py", line 446, in 
test_self_subclass
with self.assertRaises(TypeError):
^^
AssertionError: TypeError not raised

==
FAIL: test_special_method_lookup (test.test_descr.ClassPropertiesAndMethods)
--
Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_descr.py", line 2097, in 
test_special_method_lookup
runner(X())
^^^
  File "/home/serhiy/py/cpython/Lib/test/test_descr.py", line 2032, in 
do_isinstance
return isinstance(int, obj)
   
  File "/home/serhiy/py/cpython/Lib/test/test_descr.py", line 2077, in 
__getattribute__
test.fail("__getattribute__ called with {0}".format(attr))
^^
AssertionError: __getattribute__ called with __instancecheck__

--

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
keywords: +patch
pull_requests: +27789
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/29540

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Raymond Hettinger]

Raymond Hettinger  added the comment:

FWIW, I discovered the issue when experimenting with ways to use the class 
pattern in structural pattern matching.


--- Code that should work but doesn't ---

class Warm:
def __instancecheck__(cls, inst):
return inst in {'red', 'orange', 'blue'}
  
match 'red':
case Warm():  # This doesn't match but should
print('Glowing')


--- What you have to do to get it to work ---

class MetaWarm(type):
def __instancecheck__(cls, inst):
return inst in {'red', 'orange', 'blue'}

class Warm(metaclass=MetaWarm):
pass

match 'red':
case Warm():  # This matches
print('Hot')

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked on type(cls) instead of cls

[Raymond Hettinger]

Change by Raymond Hettinger :


--
title: __instancecheck__ being checked of type(cls) instead of cls -> 
__instancecheck__ being checked on type(cls) instead of cls

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked of type(cls) instead of cls

[Guido van Rossum]

Guido van Rossum  added the comment:

I'm torn. It's been like this for a very long time and Serhiy's reasoning makes 
some sense.

OTOH it *does* make more sense as it's written in the PEP. The basic idea is 
"the class has control over what is considered an instance of it" and it would 
be nice if this didn't require writing a metaclass. (Metaclasses are often 
problematic.)

As an experiment, could you implement the proposed behavior and see what fails 
in e.g. the stdlib or the test suites of the 100 most popular packages?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked of type(cls) instead of cls

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> As in most (but not all) cases of dunder methods it
> is looked up in a class, ignoring instance attributes.

That is true, but the starting point in this case is a class so the attribute 
lookup should be in that class, not its metaclass.

Guido's original code was correct and it became broken in 3.1 in the switch 
from PyObject_GetAttr(cls, name) to _PyObject_LookupSpecial(cls, 
___instancecheck__).

Also, the demonstration code I gave (that matches the documentation) used to 
work in 3.0 and prior.  It broken in 3.1.

--
nosy: +gvanrossum

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked of type(cls) instead of cls

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I think that the code is correct, and the documentation is not complete. As in 
most (but not all) cases of dunder methods it is looked up in a class, ignoring 
instance attributes.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue45791] __instancecheck__ being checked of type(cls) instead of cls

[Raymond Hettinger]

New submission from Raymond Hettinger :

Per PEP 3119, "Overloading works as follows: The call isinstance(x, C) first 
checks whether C.__instancecheck__ exists, and if so, calls 
C.__instancecheck__(x) instead of its normal implementation."

However, this doesn't work because the isinstance() has a bug introduced in 
Python 3.1 and no one ever noticed.

In abstract.c::object_recursive_isinstance(), we have:

PyObject *checker = _PyObject_LookupSpecial(cls, ___instancecheck__);

However, that function expects an instance as an argument rather than a class.

Calling typeobject.c::_PyObject_LookupSpecial() runs:

res = _PyType_LookupId(Py_TYPE(self), attrid);

Note, the Py_TYPE(self) is intended to move up from an instance to a class, but 
we're already started with a class, so it moves to the metaclass instead.

This code was correct when originally implemented but it did not have tests:

if (name == NULL) {
name = PyString_InternFromString("__instancecheck__");
if (name == NULL)
return -1;
}
checker = PyObject_GetAttr(cls, name);
if (checker == NULL && PyErr_Occurred())
PyErr_Clear();


--- Demonstration code ---

class C:
def __instancecheck__(self, inst):
raise RuntimeError(f'{self=}  {inst=}')


class P:
def __instancecheck__(self, inst):
raise RuntimeError(f'{self=}  {inst=}')


class C(P):
pass

>>> isinstance(C(), P)# Incorrectly fails to invoke __instancecheck__
True

>>> isinstance(C(), P())  # Incorrectly invokes __instancecheck__
Traceback (most recent call last):
  File "", line 1, in 
isinstance(C(), P())
  File "", line 3, in __instancecheck__
raise RuntimeError(f'{self=}  {inst=}')
RuntimeError: self=<__main__.P object at 0x107586c80>  inst=<__main__.C object 
at 0x107587100>

--
components: Interpreter Core
messages: 406187
nosy: rhettinger
priority: normal
severity: normal
status: open
title: __instancecheck__ being checked of type(cls) instead of cls
versions: Python 3.10, Python 3.11, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com