Mixin support?

[dfee]

trafficstars

1. Mixining two classes that both inherit from pyrsistent.PClass fails:

import pyrsistent

class TimestampMixin(pyrsistent.PClass):
    created_at = pyrsistent.field()
    deleted_at = pyrsistent.field()
    updated_at = pyrsistent.field()

class IDMixin(pyrsistent.PClass):
    id = pyrsistent.field()

class UserEntity(IDMixin, TimestampMixin):
    pass

produces:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-808a9f22c5af> in <module>()
      9     id = pyrsistent.field()
     10
---> 11 class UserEntity(IDMixin, TimestampMixin):
     12     pass

~/.local/share/virtualenvs/server-hvEypR1j/lib/python3.6/site-packages/pyrsistent/_pclass.py in __new__(mcs, name, bases, dct)
     20             dct['__slots__'] += ('__weakref__',)
     21
---> 22         return super(PClassMeta, mcs).__new__(mcs, name, bases, dct)
     23
     24 _MISSING_VALUE = object()

TypeError: multiple bases have instance lay-out conflict

2. Using a mixin which doesn't inherit from PClass doesn't have it's fields registered:

import pyrsistent

class TimestampMixin:
    created_at = pyrsistent.field()
    deleted_at = pyrsistent.field()
    updated_at = pyrsistent.field()

class IDMixin(pyrsistent.PClass):
    id = pyrsistent.field()

class UserEntity(IDMixin, TimestampMixin):
    pass

assert list(UserEntity._pclass_fields) == ['id']  # expecting: id, created_at, deleted_at, updated_at

Is there a suggested way to use mixins without falling back to: type('UserEntity', (pyrsistent.PClass,), namespace) where namespace is effectively a user-manifest mixin?

[tobgu]

Thanks for describing your use case!

I think the first case (eg. mixing fields from multiple PClasses into an inherited class) should be doable and seems like a reasonble use case. I haven't looked into the exact reason of the error you get but the PClass has not been designed for the mixin-scenarios that you describe so I'm not too surprised that it doesn't work (currently).

The second case is according to what I would expect. Since it is the meta class of the PClass that actually registers the fields they will not be available from the TimestampMixin. I don't think that is something I ever would want to support or encourage since the mix of a mutable and immutable ancestors could produce very confusing results.

I would be happy to discuss option one further and take a PR on if you want to get your hands dirty. I don't have a lot of time myself to spend on new functionality in Pyrsistent at the moment.

Regarding your request for other ways of supporting mixin-like functionality I don't have any better options of the top of my head. I'll let you know if I come up with something.

[dfee]

It turns out that the errors comes from inheriting from two classes which have __slots__. There would need to be some interesting trickery to maintain the class hierarchy.

How would you proceed?

[tobgu]

I see, I wasn't aware of that. Seems like a hard thing to work around.

I would suggest adding a new, special and explicit, class field which would list the "base classes" that you would like to "inherit" fields from.

class Bar(PClass):
   bar_field = field()

class Baz(PClass):
   baz_field = field()

class Foo(PClass):
   __fields_from__ = (Bar, Baz)
   foo_field = field()

The meta class would then take care of including all fields from Bar and Baz into Foo. This way it would also be super obvious what the intention is and not convolute the inheritance.

I'm slightly reluctant to call this something related to mixin. My understanding of the term is that it's normally used to add behaviour, but not state, to a class. This seems to be the other way around?

[tobgu]

@dfee Is this something you would like to see implemented? If so would you have the time to make a PR?

At the moment I don't do any feature development on this project unless I need the functionality myself because of lack of time. I do fix bugs, take PRs and release new versions when needed though!

[dfee]

@tobgu I don't think I'll be able to contribute to this as it'd be a pretty massive / fundamental code change.

For now, I think the answer is for users to work-around the problem (as you described).

[tobgu]

That's OK, I'll leave the issue open, label it as an enhancement that someone can grab if they like. There are a bunch of similar issues that I myself don't have time to work on but which I happily would accept PRs for.

[gelatinouscube42]

Is this behavior (of type 1 above) supported now?

I've been trying various tricks to get inheritance of fields to work, and this seems like it would have been an answer for my use case. Not sure where to start going about tooling around in the code itself to implement it if not yet there.