This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Lib/types.py nit: isinstance != PyType_IsSubtype
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: gmt, yselivanov
Priority: normal Keywords: patch

Created on 2014-11-29 20:38 by gmt, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
fix_types_calculate_meta.patch gmt, 2014-11-29 20:38 More correct implementation of types._caclculate_meta review
fix_types_calculate_meta_v2.patch gmt, 2014-12-03 03:06 v2: More correct implementation of class creation APIs in Lib/types.py review
fix_types_calculate_meta_v3.patch gmt, 2014-12-03 12:58 v3: Just adds a commit message review
test_virtual_metaclasses.patch gmt, 2014-12-03 13:02 Tests verifying that type machinery really works this way review
test_virtual_metaclasses_in_types_module.patch gmt, 2014-12-03 13:04 Tests verifying that types.py matches type machinery review
test_virtual_metaclasses_in_types_module_v2.patch gmt, 2014-12-04 05:31 Tests verifying that types.py implementation matches native type machinery review
Messages (7)
msg231871 - (view) Author: Greg Turner (gmt) * Date: 2014-11-29 20:38
Kinda trivial but...

Something like the enclosed, untested patch would seem to make new_class work a bit more like type_new.

To be explicit, the difference is whether or not to respect virtual subclasses.

So, for example, as implemented, we can implement __subclasscheck__ on a 'AtypicalMeta' metaclass to create an 'Atypical' class whose __mro__ is (ATypical, object) -- note, no 'type' -- but for which issubclass(type, ATypical) is true.

If I'm not mistaken, this disguise will suffice sneak our psuedo-metatype past new_class (but not type.__new__, so we still get the same error message, and the universe does not implode on itself as planned...)

In my case,the only sequela was that the fantasy of my very own type-orthogonal graph of "foo"-style classes in 3.x was first subtly encouraged and then dashed against the Cpython rocks... (just kidding, sort-of).
msg231980 - (view) Author: Greg Turner (gmt) * Date: 2014-12-02 08:07
Also:

In types.prepare_class, here is what currently happens:

we let x be, in order of preference:
  (a) the explicitly given type if, one is given, else
  (b) type(bases[0]), if it exists, else
  (c) type

and then, if isinstance(x, type), we run _calculate_bases.

In python 2, I think something like this really does happen, although, perhaps "isinstance(x, type)" should have been "issubclass(x, type)" to correctly capture how python 2 does it.

In particular, I think we can stick a Callable in there as bases[0], and then any old crazy list of objects as base classes, and it will call our Callable, although if we don't do something about our crazy base classes, it will still break later (something like that... I don't remember exactly, except that the first base is definitely special somehow).

But in python 3, if I'm reading the C code correctly, I don't think the first base class receives any "special" handling, and the cpython-equivalent to _calculate_bases /always/ happens, suggesting that any non-descending-from-type metaclass is expected to have removed itself from the picture before type_new is invoked.

So maybe more minor re-factoring is needed to get it all straightened out.  My brain is kind of fried from looking at it, I'll try again later.
msg232067 - (view) Author: Greg Turner (gmt) * Date: 2014-12-03 03:06
> perhaps "isinstance(x, type)" should have been "issubclass(x, type)" to correctly capture how python 2 does it.

Actually, scratch that -- its a brain fart.

IIUC new_class was created because, at the time, there was no exposed turnkey method to execute the PEP3115 protocol... if that's right, is it worth considering, now that __build_class__ is exposed, a reimplementation like:

def _pass(ns): pass

def new_class(name, bases=(), kwds={}, exec_body=_pass):
    __build_class__(exec_body, name, *bases, **kwds)

?  But this is the wrong place to ask.

...

BTW, since my original post I've figured out a bunch of things, or think I have:

* new_class should model __build_class__

* __build_class__ need not model type_new in terms of what it allows.

I think point one is what was confusing me before.

It is perhaps worth asking, if the differences between type_new and __build_class__ make sense, given that python 3's type_new is stricter than python 2's?  I think I've figured that out too:

A given (metatype, bases) tuple may be sensibly used to implement the PEP3115 protocol only if either:

  A) it can be proved that type_new will return NULL if it
     is invoked like type_new(metatype, name, bases,...), or,

  B) it can be proved that type_new, when invoked like
     type_new(metatype, name, bases, ...), will use metatype,
     and not some other (e.g., more derived) type, as the tp_base
     of the thing it creates, in the event that it does not return NULL.

If the above is falsified, we pretty clearly have a situation where build_type has used the wrong metaclass.

But the fact that __build_class__ is more liberal than type_new is not a problem from the perspective of type_new -- indeed it's a feature, and I'm now satisfied that the above constraint holds.

I've attached a new patch, this time I tested it.
msg232074 - (view) Author: Greg Turner (gmt) * Date: 2014-12-03 12:58
Just added a commit message.
msg232075 - (view) Author: Greg Turner (gmt) * Date: 2014-12-03 13:02
Here are some tests for this... first, some tests that pass right now and demonstrate, I think, correctness of the second batch of tests.
msg232077 - (view) Author: Greg Turner (gmt) * Date: 2014-12-03 13:04
And some analogous tests for types.py, which don't pass without fix_types_calculate_meta_v3.patch.
msg232104 - (view) Author: Greg Turner (gmt) * Date: 2014-12-04 05:31
Fixed a trivial typo in test_virtual_metaclass_in_types_module.patch
History
Date User Action Args
2022-04-11 14:58:10adminsetgithub: 67157
2015-10-24 15:02:13serhiy.storchakasetnosy: + yselivanov
stage: patch review

versions: + Python 3.6
2014-12-04 05:31:25gmtsetfiles: + test_virtual_metaclasses_in_types_module_v2.patch

messages: + msg232104
versions: + Python 3.5
2014-12-03 13:04:20gmtsetfiles: + test_virtual_metaclasses_in_types_module.patch

messages: + msg232077
2014-12-03 13:02:25gmtsetfiles: + test_virtual_metaclasses.patch

messages: + msg232075
2014-12-03 12:58:17gmtsetfiles: + fix_types_calculate_meta_v3.patch

messages: + msg232074
2014-12-03 03:17:39gmtsettitle: types._calculate_meta nit: isinstance != PyType_IsSubtype -> Lib/types.py nit: isinstance != PyType_IsSubtype
2014-12-03 03:06:36gmtsetfiles: + fix_types_calculate_meta_v2.patch

messages: + msg232067
2014-12-02 08:07:59gmtsetmessages: + msg231980
2014-11-29 20:38:34gmtcreate