Codecov Report

Merging #2534 (fe81bf2) into main (f818083) will increase coverage by 0.04%.
The diff coverage is 95.83%.

numpy issue numpy/numpy#22876 and PR numpy/numpy#23113

Oh, so my reading of this is that

• NumPy only introduced __slots__ = ()
• mixins without __slots__ are not mixable into classes with __slots__ (worse: Python just ignores the subclassed __slots__)
• a superclass/mixin with __slots__ = () is unrestrictive about what slots downstream classes define.

By that logic, the NumPy developers concluded that there could be no downside to it. In #2533, we seem to have discovered a downside: __class__ can't be assigned to by other classes that don't define __slots__ because they have a different memory model.

Maybe a different solution to this is to define __slots__ for ak.Array and ak.Record? That would put them in the same memory model as NDArrayOperatorsMixin-with-__slots__. If we do that, would that require downstream libraries from Awkward (i.e. Coffea and Vector) to also define __slots__ in their behavior classes?

We don't want an infectious implementation requirement to spread to all downstream libraries that use Awkward, especially if the error message is cryptic, like

TypeError: __class__ assignment: 'MyAwesomeBehavior' object layout differs from 'NDArrayOperatorsMixin'

If defining __slots__ for ak.Array and ak.Record infects downstream libraries with the need to define __slots__ on their behaviors, then let's define our own NDArrayOperatorsMixin without __slots__. (At worst, downstream libraries will try to use __slots__ and Python will ignore their performance hints.)

If defining __slots__ for ak.Array and ak.Record has no implications for downstream libraries, let's define __slots__ for ak.Array and ak.Record.

The way that the layout checker works is that it has a notion of type-supertype "compatibility", and it attempts to find a compatible common base.

To generate our mixin classes for both records and arrays, we do something like

class ArrayImpl(MixinType, ak.Array):
    ...

The logic checking type-supertype compatibility for multiple inheritance walks down the tp_base (or __base__ in Python) slot, and checks whether it is "compatible" with the type. If the child has a __dict__ and the parent does not, this is False.

Let's imagine that we add slots to ak.Array and the generated classes for records and arrays. If the user doesn't define __slots__, we'll end up with a dict, and the compatibility check fails. Conversely, without __slots__, the base for the mixin (ArrayImpl) is object, whilst for ak.Array the base is NDArrayOperatorsMixin, which also fails.

Of the two approaches, it's probably safer to assume that the user doesn't define __slots__ for these mixin classes, because doing so would currently fail:

import awkward as ak


behavior = {}


class Point:
    __slots__ = ()


class MyPoint(Point, ak.Array):
    ...


behavior["*", "Point"] = MyPoint

ak.operations.zip(
    {
        "x": ak.Array([1, 1]),
        "y": ak.Array([1, 1]),
    },
    with_name="Point",
    behavior=behavior,
)

For the same reasons outlined above: MyPoint.__base__ is Point, which is the final "compatible base" for MyPoint, and is not the same base as that of ak.Array.

I think, therefore, the safest immediate step is to drop the slots, by using our own mixin. For our ecosystem, I think there's lesser benefit to __slots__ usage; we don't expect high-frequency usage of array attributes to dominate over the array kernels. I might be wrong, though.

I think, therefore, the safest immediate step is to drop the slots, by using our own mixin. For our ecosystem, I think there's lesser benefit to __slots__ usage; we don't expect high-frequency usage of array attributes to dominate over the array kernels. I might be wrong, though.

Let's go with that. I was also thinking that __slots__ aren't going to help much for ak.Array, considering that it contains so many large, nested, tree-like objects.

I've taken your above comment as approval for merging!