[issue35083] Fix documentation for __instancecheck__
New submission from Joy Diamond <python.gem@gmail.com>: This is a request to fix the documentation for __instancecheck__. Please add the following: """ (Note that any object `x` is always considered to be an instance of `type(x)`, and this cannot be overridden.) """ Consider the following program: class M(type): def __instancecheck__(m, t): print('instancecheck(%s, %s)' % (m, t)) return False # LIE! Test = M('Test', ((object,)), {}) something = Test() print('Does *NOT* call __instancecheck__:') print('isinstance(something, Test): %s' % isinstance(something, Test)) print() print('Does call __instancecheck__:') print('isinstance(0, Test): %s' % isinstance(0, Test)) Under python 2, python 3, and pypy, in all cases, the first examples does *NOT* call __instancecheck__. You can see the optimization here: https://github.com/python/cpython/blob/master/Objects/abstract.c#L2397-L2405 Which reads: int PyObject_IsInstance(PyObject *inst, PyObject *cls) { _Py_IDENTIFIER(__instancecheck__); PyObject *checker; /* Quick test for an exact match */ if (Py_TYPE(inst) == (PyTypeObject *)cls) return 1; I'm fine with the optimization -- I am not suggesting to get rid of it. I just want the documentation to match the actual implementation. The following documentation needs to be fixed: https://docs.python.org/2/reference/datamodel.html https://docs.python.org/3/reference/datamodel.html https://www.python.org/dev/peps/pep-3119/ It took me half an hour to figure out why my version of __instancecheck__ was not working, as I tried to test it using the super simple code above ... One of the best things about python is how accurate and consistent the documentation is. This request is to keep these high standards. ---------- assignee: docs@python components: Documentation messages: 328658 nosy: docs@python, joydiamond priority: normal severity: normal status: open title: Fix documentation for __instancecheck__ type: enhancement versions: Python 2.7, Python 3.8 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
Karthikeyan Singaravelan <tir.karthi@gmail.com> added the comment: python-ideas thread for the issue : https://mail.python.org/pipermail/python-ideas/2018-October/054335.html ---------- nosy: +xtreak _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
Julien Palard <julien+python@palard.fr> added the comment: Currently [1] there's no concensus about: - Is this a documentation issue and we should document the optimisation? - Is the optimisation over-zealeous? [1]: https://mail.python.org/pipermail/python-ideas/2018-October/054342.html ---------- nosy: +mdk _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
Change by Caleb Donovick <donovick@cs.stanford.edu>: ---------- nosy: +donovick _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
Change by M. Eric Irrgang <ericirrgang@gmail.com>: ---------- nosy: +eirrgang _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
M. Eric Irrgang <ericirrgang@gmail.com> added the comment: The optimization appears to have been made without this level of discussion. The change to PEP 3119 was likely overlooked. The intended PEP 3119 behavior seems clear and reasonable. r61575 was a small part of the SVN merge that became git commit d5e2b6f3bcef9fea744bef331ad7278052223f11 Suggest reverting the `/* Quick test for an exact match */`. If either the optimization is important, or the documented PEP 3119 behavior should be revised, suggest re-raising the issue as a proposed revision to PEP 3119. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
M. Eric Irrgang <ericirrgang@gmail.com> added the comment: Actually, it looks like performance concerns were raised as issues [2303](https://bugs.python.org/issue2303) and [2534](https://bugs.python.org/issue2534). For #2534, the issue was different. For issue #2303, behavior-changing optimization was proposed. The first comment describes the cost of requiring a check to whether `__instancecheck__` is present (though I don't see what the comment refers to in 1b7f891f416830d0c46ca1c9e1bfe62f05cda655, which is the commit containing the cited r58099, as best I can tell). However, the discussion may have ended prematurely when the optimization https://bugs.python.org/file9742/isinst.diff slipped in through a different change. See https://bugs.python.org/issue2303 for discussion. For what it's worth, Guido (author of PEP 3119) did not seem to object to https://codereview.appspot.com/504/diff/1/Objects/abstract.c and no one seems to have discussed the apparent inconsistency with PEP 3119. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue35083> _______________________________________
participants (5)
-
Caleb Donovick
-
Joy Diamond
-
Julien Palard
-
Karthikeyan Singaravelan
-
M. Eric Irrgang