plum icon indicating copy to clipboard operation
plum copied to clipboard

make @parametric work with classes with arbitrary metaclasses

Open PhilipVinc opened this issue 2 years ago • 4 comments

I am experimenting with https://github.com/cgarciae/treeo and plum, and found out that since both inject meta classes they don't work together.

This PR makes plum work with classes which already have a meta class by injecting CovariantMeta as a superclass of the existing meta class, and by redefining __call__ (essentially the constructor) to infer the parameters before calling into the existing class.

I'm surprised this was so easy to implement!

PhilipVinc avatar Dec 04 '21 17:12 PhilipVinc

Coverage Status

Coverage remained the same at 100.0% when pulling a0df7ac5a61443dbcfb1438b4fa2d129e0f86b10 on PhilipVinc:pv/parametricmeta into b521badf3447d61fc06ddb1956ac9e658639e9ce on wesselb:master.

coveralls avatar Dec 04 '21 17:12 coveralls

Thank you, @PhilipVinc!! This really is excellent—a strict improvement. Merging this now and releasing a new version. :)

wesselb avatar Dec 09 '21 19:12 wesselb

Actually, you've still marked this as a draft. The PR looks complete to me. Are you happy to have this merged?

wesselb avatar Dec 09 '21 19:12 wesselb

Yeah, I think I wanted to add a test to check for what the fix issue in __init_subclass__ commit fixed. I found out about it in a very exotic case that arises when you combine @parametric with classes that auto-register themselves with jax, but it's a real issue of plum,.

I'll add a test soon

PhilipVinc avatar Dec 10 '21 17:12 PhilipVinc