Skip to content
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

Support inheritance for __extends__() #34

Open
davidjlloyd opened this issue Jan 23, 2018 · 1 comment
Open

Support inheritance for __extends__() #34

davidjlloyd opened this issue Jan 23, 2018 · 1 comment

Comments

@davidjlloyd
Copy link
Contributor

Currently the __extends__() method for a Capnpy struct only supports simple classes with no inheritance. Attempting to use a class with inheritance results in a struct with only the methods defined on the class directly available to the struct; any inherited methods are lost.

The method should support classes with inheritance in a sensible way. All methods present on the class should be available on the struct. This will allow developers to avoid duplication of code when they need to share methods across these classes.

@antocuni
Copy link
Owner

__extend__ was designed with this precise use case in mind:

@MyStruct.__extend__
class MyStruct:
    x = 42
    def foo(self):
        ...

what it does is simply to copy the items of the new class body inside the original one. It's just syntactic sugar for this:

def foo(self):
    ...
MyStruct.x = 42
MyStruct.foo = foo

(apart the fact that __extend__ has special support for modifying also classes defined in C).

I am not sure what "supporting inheritance" would mean. E.g.:

@MyStruct.__extend__
class MyStruct(MyBase):
    pass

do you expect issubclass(MyStruct, MyBase) (and similarly for isinstance(...)) to be True? This is simply not possible, because when __extend__ is run, MyClass has already been created and you can no longer modify it.

We could maybe try to mimic some of the effects of inheritance, e.g. by walking through MyBase.__mro__ and manually copying all the methods we find. However, I am a bit skeptical that this is a good idea, because it opens a can of worms: there will always be corner cases which we can't fully support: e.g., if you modify MyBase after you run __extend__(), the new changes won't be reflected inside MyStruct. Also, this will surely break in presence of multiple inheritance and super().

I agree that there are cases in which such a scheme is useful to avoid code duplicating, but I don't think that such complicated/half-broken logic should be put inside capnpy. Depending on the complexity of the code, there are alternative solutions which IMHO work equally well:

  1. manually call __extend__ multiple times to get all the methods you need; e.g.:
MyStruct.__extend__(A)
MyStruct.__extend__(B)
MyStruct.__extend__(C)
  1. write a custom helper outside capnpy which walks the __mro__ and copy the methods as needed. The advantage is that being outside capnpy, you can write the minimum amount of logic to solve your specific needs, without further complications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants