Skip to content

Inheritance for native types is not supported #1159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pfalcon opened this issue Mar 16, 2015 · 11 comments
Open

Inheritance for native types is not supported #1159

pfalcon opened this issue Mar 16, 2015 · 11 comments
Labels
enhancement Feature requests, new feature implementations

Comments

@pfalcon
Copy link
Contributor

pfalcon commented Mar 16, 2015

Default load_attr implementation (as used for all native types) doesn't take type inheritance (->bases_tuple) into account. I'm working on OrderedDict implementation, and it inherits most of its methods from dict, but adds few (well, 1) new method. Of course, this new method can be ignored, and old trick of putting the same .locals_dict to both types can be used. But I wanted to look into making truly inheritable methods, which requires setting custom .load_attr type method. And we already have implementation of this method which takes inheritance into account - for user classes/instances. But trying to use it, it assumes that object whose attr is being looked up is instance, so making it work with native objects means adding even more conditions to existing bunch of those (and which are already pretty mind-boggling). Another alternative is to make clean, "optimized" inheritance-friendly .load_attr for native types, but that means code duplication (and refactoring of existing code paths, factoring out functions, which means more stack usage, etc.)

Any thoughts?

@pfalcon pfalcon added the enhancement Feature requests, new feature implementations label May 4, 2015
nickzoic pushed a commit to nickzoic/micropython that referenced this issue Sep 6, 2018
When UART timeout of zero is given, make read() return data already a…
@amirgon
Copy link
Contributor

amirgon commented Nov 27, 2018

I don't understand how this never got priority since 2015.
When providing Micropython API to an existing library, I have to duplicate all methods of base classes for each sub class instead of simply specify the base class. I get huge locals_dicts with duplicated data. Feels like the mechanism for extending Micropython is crippled.

@stinos
Copy link
Contributor

stinos commented Nov 28, 2018

I have to duplicate all methods of base classes

Not really, there are basic tricks which minimize boilerplate to the point you can't really speak of duplication anymore. For example:

def CopyAttributes(src, dst, names=None):
  if not names:
    names = (e for e in dir(src) if not e.startswith('__'))
  for entry in names:
    setattr(dst, entry, getattr(src, entry))

class Derived:
  def __init__(self):
    self.base = Base()
    CopyAttributes(self.base, self)

@amirgon
Copy link
Contributor

amirgon commented Nov 28, 2018

@stinos I'm not providing python code. I'm providing an extension to micropython by adding a module in C.
Do you suggest I post-process every object I add from C with python code? This isn't very scalable or convenient. May also have performance/memory penalty.

@stinos
Copy link
Contributor

stinos commented Nov 28, 2018

The same principle can also be applied in C if you're not using a const dict.

@amirgon
Copy link
Contributor

amirgon commented Nov 28, 2018

@stinos Please explain how to achieve this when adding a module. I tried setting .parent field of mp_obj_type_t but this does not work for native (C) objects.

@stinos
Copy link
Contributor

stinos commented Nov 28, 2018

I don't have an example ready but thinking of it, doing this in C is actually a bit different but less bloated as you don't need storage for each method. The idea is: you implement the type's attr function and when MicorPython asks you for an attribute you look it up in your class first, and if not found you look in your 'base' class instead and return it if found. Here's some untested pseudocode thrown together:

static void mytype_make_new(...) {
  //all the usual stuff, plus keep a 'base' class object
  self->myBasePtr = mybase_make_new(...);
}

static void myattr(mp_obj_t self_in, qstr attr, mp_obj_t* dest) {
  mytype *self = (mytype*) self_in;
  if(*dest == MP_OBJ_NULL) { //called to load attribute
      mp_obj_t myOwnAttribute = mp_map_lookup(self->base.type->locals_dict->map, new_qstr(attr), MP_MAP_LOOKUP);
      if(myOwnAttribute != MP_OBJ_NULL) {
        dest[0] = elem->value;
        dest[1] = self_in;
      }
      //we don't have that attribute, maybe base does? just forward call
    self->myBasePtr->base.type.attr(self->myBasePtr, attr, dest);
  } else {
    //do whatever needed to store attribute
  }
}

const mp_obj_type_t my_type = {
    //the usual stuff
    .make_new = mytype_make_new,
    .attr = myattr
};

@amirgon
Copy link
Contributor

amirgon commented Nov 28, 2018

@stinos Thanks for your example, I'll give it a try.
Don't you think this is something worth supporting as part of Micropython, instead of everyone implement the same functionality again and again?
Also I guess that in REPL, these functions will not be completed with TAB, will not appear in help etc...

@peterhinch
Copy link
Contributor

Apologies if I'm missing the point here but it seems to me that providing generalised inheritance of built in types is considerably more complex than copying attributes. Consider properties like mutability, the ability to support item assignment, the ability to iterate over the instance, support for the buffer protocol and doubtless more. These may or may not be present in the base class.

@amirgon
Copy link
Contributor

amirgon commented Nov 29, 2018

@stinos @peterhinch It's worth noting that at least in Python this is already covered. (PEP253, an example)

@stinos
Copy link
Contributor

stinos commented Nov 29, 2018

Don't you think this is something worth supporting as part of Micropython, instead of everyone implement the same functionality again and again?

Yes of course, that's also why this issue exists.

Apologies if I'm missing the point here but it seems to me that providing generalised inheritance of built in types is considerably more complex

You're not missing any point, the example shown isn't generalised at all.

in Python this is already covered

Well I surely hope there are shortcuts not shown in those example then, instead of having to copy that 40-line type definition :P

@dpgeorge
Copy link
Member

See #4360 for a solution to this issue. It just does single inheritance but I think that is enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests, new feature implementations
Projects
None yet
Development

No branches or pull requests

5 participants