py/objtype: Add type.bases attribute. #5139

nevercast · 2019-09-24T05:11:45Z

Proposal: Add __bases__ attribute to types so that the inheritance heirachy can be inspected. Related to #5106 and #4368

I have not implemented .__base__ nor have I made any changes to anything MRO related. This is simply a read-only attribute that supports single and multiple inheritance.

For me, the intent of this addition is so I can type check objects in my application to ensure they inherit from a common base class.

It should be noted that I would like to add documentation and tests for this, once we finalize the draft. I'm not proposing we ignore housekeeping.

Example:

>>> class A:
...     pass
>>> class B(object):
...     pass
>>> class C(B):
...     pass
>>> class D(C, A):
...     pass
>>> A.__bases__
(<class 'object'>,)
>>> B.__bases__
(<class 'object'>,)
>>> C.__bases__
(<class 'B'>,)
>>> D.__bases__
(<class 'C'>, <class 'A'>)

nevercast · 2019-09-24T06:09:53Z

Sorry about the force push there, fixed a bad pointer NULL comparison (which should fix the preexisting tests). Didn't wish to introduce another commit for that, though it might have been cleaner and more obvious.

jimmo

I don't have strong feelings about whether MICROPY_CPYTHON_COMPAT is the right flag to gate this feature. It's on by default, and only disabled in the really minimal ports. The whole feature costs 84 (or maybe 72, see comment) bytes on STM32 (e.g. PYBV11).

Based on your comment, is it a feature you'd only want for unit testing? In which case perhaps only the Unix port needs it (or specifically your board config).

py/objtype.c

nevercast · 2019-09-24T06:25:59Z

Based on your comment, is it a feature you'd only want for unit testing? In which case perhaps only the Unix port needs it (or specifically your board config).

This is actually a feature I'm using on the microcontroller since I don't have different code generation between tests and application at this stage.

nevercast · 2019-09-24T06:55:36Z

Code size changes:

Port	Before	After	Bytes
Unix	496856	496944	+88
PYBV11	396832	396900	+68
ESP32	1202048	1202112	+64

I tried MP_OBJ_FROM_PTR((self->parent ? self->parent : &mp_type_object)); and it was no smaller.

nevercast · 2019-09-24T07:21:56Z

Squish squash.

nevercast · 2019-09-24T08:20:47Z

Squash tests.

tests/basics/class_bases.py.exp

dpgeorge · 2019-09-27T12:35:33Z

It looks like object needs special handling in that its __bases__ is the empty tuple. I think it's important to get this correct because it can be used to signal the end of a search when recursively looking up __bases__ of a type.

One way to fix this would be to modify all built-in types (eg str, bytes, dict, etc) so that they explicitly have .parent = &mp_type_object. Currently the default is .parent = NULL which implies that the parent is actually object. Making it explicit might even simplify some of the inheritance code elsewhere.

nevercast · 2019-09-27T22:06:26Z

You are correct, I didn't consider this case. I'll make some tests for it and go about fixing it. Can't say I'll get it done this weekend but I will look in to it.

nevercast · 2019-10-18T01:28:46Z

One way to fix this would be to modify all built-in types (eg str, bytes, dict, etc) so that they explicitly have .parent = &mp_type_object. Currently the default is .parent = NULL which implies that the parent is actually object.

Another way would be to special case object to have an empty tuple

Making it explicit might even simplify some of the inheritance code elsewhere.

This is a good motivation to go with the longer route but do we want to include that kind of change in this patch?

nevercast · 2019-10-18T01:46:10Z

Alright - I've branched this pull request in to two streams, this is the simplist approach, treat object special. If that's the only edge case then this is the easiest way to cover it.

I'm now exploring the other route, redefining what it meants for .parent=NULL.

nevercast · 2019-10-18T01:49:51Z

Keeping track of branches here for review purposes.

This pull request: https://github.com/nevercast/micropython/tree/proposal/bases

The simplist stream: https://github.com/nevercast/micropython/tree/proposal/bases_special
The more technical stream: https://github.com/nevercast/micropython/tree/proposal/bases_technical

This pull request before branching https://github.com/nevercast/micropython/tree/proposal/bases_clean

nevercast · 2019-10-18T02:10:03Z

Pushed fix for bad compare between pointers.

nevercast · 2019-10-18T02:11:24Z

Just while I'm doing this, I notice it starts to get to be a pretty bloated change.

extern const mp_obj_type_t mp_type_int;
extern const mp_obj_type_t mp_type_str;
extern const mp_obj_type_t mp_type_bytes;
extern const mp_obj_type_t mp_type_bytearray;
extern const mp_obj_type_t mp_type_memoryview;
extern const mp_obj_type_t mp_type_float;
extern const mp_obj_type_t mp_type_complex;
extern const mp_obj_type_t mp_type_tuple;
extern const mp_obj_type_t mp_type_list;
extern const mp_obj_type_t mp_type_map; // map (the python builtin, not the dict implementation detail)
extern const mp_obj_type_t mp_type_enumerate;
extern const mp_obj_type_t mp_type_filter;
extern const mp_obj_type_t mp_type_deque;
extern const mp_obj_type_t mp_type_dict;
extern const mp_obj_type_t mp_type_ordereddict;
extern const mp_obj_type_t mp_type_range;
extern const mp_obj_type_t mp_type_set;
extern const mp_obj_type_t mp_type_frozenset;
extern const mp_obj_type_t mp_type_slice;
extern const mp_obj_type_t mp_type_zip;
extern const mp_obj_type_t mp_type_array;
extern const mp_obj_type_t mp_type_super;

A lot of types to add default .parent= to.
Also, interestingly, bool has a parent on int

Should I go and make all the built-in types like CPython ? I've started writing tests checking that all the builtins .bases match, but I'm wondering if we should default to .parent = mp_type_object and just have an edge case for instantiated classes and <type 'object'>

dpgeorge · 2019-10-18T03:01:47Z

Another way would be to special case object to have an empty tuple

I do like that idea, it would eliminate the special test for object in the PR here as it stands. But I don't think it'll work because the code assumes that, if MICROPY_MULTIPLE_INHERITANCE is disabled, .parent is either NULL or a type, and the empty tuple is neither of those.

Just while I'm doing this, I notice it starts to get to be a pretty bloated change.

Yes indeed it does...

but I'm wondering if we should default to .parent = mp_type_object and just have an edge case for instantiated classes and <type 'object'>

Yes I think that's the best way forward at this point, to just make small improvements.

So, as it currently stands I think this PR is good to go in. Later on we can improve/optimise things by explicitly populating the .parent entry for all types.

nevercast · 2019-10-18T03:14:11Z

So, as it currently stands I think this PR is good to go in. Later on we can improve/optimise things by explicitly populating the .parent entry for all types.

I'm happy for this PR to land in master as it is now, and I can continue exploring better solutions and raise incremental changes in the future.

nevercast · 2019-10-18T03:19:11Z

One last squash to drop the extra commit message.

nevercast · 2019-10-18T03:19:56Z

py/objtype.c

@@ -1014,6 +1014,21 @@ STATIC void type_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) {
            dest[0] = MP_OBJ_NEW_QSTR(self->name);
            return;
        }
+        if (attr == MP_QSTR___bases__) {
+            mp_obj_t parent_obj = self->parent ? MP_OBJ_FROM_PTR(self->parent) : MP_OBJ_FROM_PTR(&mp_type_object);
+            if (self == &mp_type_object) {


These two blocks can be swapped. I'll do that now.

py/objtype.c

dpgeorge · 2019-10-18T04:21:52Z

Thanks, merged in 8f9e2e3 with a minor change to the test to skip if the feature is not enabled.

nevercast · 2019-10-18T04:23:47Z

skip if the feature is not enabled.

Whoops. Thank you!

dpgeorge · 2019-10-18T04:30:55Z

skip if the feature is not enabled.

Whoops. Thank you!

To help with this in the future see #5227

Move OneWire to `onewireio`

This addendum to micropython#5139 allows actually turning off onewireio. (Not currently used by any board.)

nevercast force-pushed the proposal/bases branch from c539a7f to 0055b73 Compare September 24, 2019 06:08

jimmo reviewed Sep 24, 2019

View reviewed changes

py/objtype.c Outdated Show resolved Hide resolved

py/objtype.c Outdated Show resolved Hide resolved

nevercast force-pushed the proposal/bases branch from 3cfd9ac to 33cccab Compare September 24, 2019 07:21

nevercast marked this pull request as ready for review September 24, 2019 08:14

nevercast force-pushed the proposal/bases branch from f12dadf to 606f5bd Compare September 24, 2019 08:20

dpgeorge reviewed Sep 26, 2019

View reviewed changes

tests/basics/class_bases.py.exp Outdated Show resolved Hide resolved

dpgeorge added the py-core Relates to py/ directory in source label Sep 26, 2019

nevercast force-pushed the proposal/bases branch from 606f5bd to 13ec0ba Compare September 27, 2019 04:48

dpgeorge mentioned this pull request Oct 16, 2019

Add .__base__ #4368

Closed

nevercast force-pushed the proposal/bases branch from 13ec0ba to 6a90a05 Compare October 18, 2019 01:45

nevercast force-pushed the proposal/bases branch from 6a90a05 to 14ad0ed Compare October 18, 2019 02:09

nevercast force-pushed the proposal/bases branch from 14ad0ed to f69490a Compare October 18, 2019 03:19

nevercast commented Oct 18, 2019

View reviewed changes

nevercast force-pushed the proposal/bases branch from f69490a to c664908 Compare October 18, 2019 03:21

dpgeorge reviewed Oct 18, 2019

View reviewed changes

py/objtype.c Outdated Show resolved Hide resolved

py/objtype: Add type.__bases__ attribute.

c6c6918

nevercast force-pushed the proposal/bases branch from c664908 to c6c6918 Compare October 18, 2019 03:28

dpgeorge closed this Oct 18, 2019

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Aug 13, 2021

Merge pull request micropython#5139 from tannewt/onewireio

ffb9ecf

Move OneWire to `onewireio`

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Aug 24, 2021

Fix linker error when ONEWIREIO = 0

d79a9e0

This addendum to micropython#5139 allows actually turning off onewireio. (Not currently used by any board.)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

py/objtype: Add type.bases attribute. #5139

py/objtype: Add type.bases attribute. #5139

nevercast commented Sep 24, 2019 •

edited

Loading

nevercast commented Sep 24, 2019

jimmo left a comment

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

dpgeorge commented Sep 27, 2019

nevercast commented Sep 27, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

dpgeorge commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast Oct 18, 2019

dpgeorge commented Oct 18, 2019

nevercast commented Oct 18, 2019

dpgeorge commented Oct 18, 2019

py/objtype: Add type.__bases__ attribute. #5139

py/objtype: Add type.__bases__ attribute. #5139

Conversation

nevercast commented Sep 24, 2019 • edited Loading

nevercast commented Sep 24, 2019

jimmo left a comment

Choose a reason for hiding this comment

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

nevercast commented Sep 24, 2019

dpgeorge commented Sep 27, 2019

nevercast commented Sep 27, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

dpgeorge commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast commented Oct 18, 2019

nevercast Oct 18, 2019

Choose a reason for hiding this comment

dpgeorge commented Oct 18, 2019

nevercast commented Oct 18, 2019

dpgeorge commented Oct 18, 2019

py/objtype: Add type.bases attribute. #5139

py/objtype: Add type.bases attribute. #5139

nevercast commented Sep 24, 2019 •

edited

Loading