Codejam#6 : Inheritance based vs composition based

[samuelgarcia]

In codejam#6, we add long discussion about getting back to composition based for neo objects with data (AnalogSignal, SpikeTrain, Epoch, Event).

Many would prefer composition instead of inheritance:

• easier to manage deferred loading.
• sig1 + sig2 magic is not so much used

Other prefer keep with inheritance:

• easier to maintain this sig1+sig2. With inheritence it just works.

The assemblies decision is to delayed this 0.5 and keep with inheritence for 0.4.

[toddrjen]

It isn't just addition, numpy provides a lot of methods (sum, mean, clip, min, max, round, etc.) that would either be lost or would need to be re-implemented.

One problem with the current approach is that neither pytables nor quantities will work with lazy-loading numpy arrays. However, if we are looking long-term, if these tools do not fit our needs, then perhaps they are not the correct tools for the job. quantities in particular appears to be unmaintained. It hasn't seen a release since 2011, and hasn't seen a commit in 5 months.

[samuelgarcia]

Andrew idea was to have internal neo.units that would rely on quantities in a first step but that we could change to others modules.

[rproepp]

Yes, moving away from quantites could be a good idea. I don't think that abstracting units into a neo.units module would work though, it would probably be as much work as implementing units ourselves.

So, what could we do? Maybe we could "adopt" quantities, since we seem to have the more active community now? Or switch to another package and communicate with the authors from the beginning to ensure sustainability? Or create or own units, possibly based on an existing package?

Also, I don't think it is a good idea to have two consecutive API breaking releases. So, I would strongly prefer to settle the inheritance vs. composition question before 0.4.0

[toddrjen]

So what is the final decision here? Should we use inheritance or composition?

[apdavison]

two consecutive API breaking releases

A switch to composition should not break the API in major ways, should it? (there will doubtless be small abstraction leakages) This is a large part of the potential pain of moving to composition, we would have to reimplement most/all of the numpy.array interface.

Also, until Neo reaches 1.0, I think we should be expecting to break the API in some way at every release.

I vote for sticking with inheritance for 0.4

[toddrjen]

I think keeping a consistent type for all data classes is required, so I think we should either switch eventarray and epocharray to an array subclass or convert analogsignal, irregularlysampledsignal, and spiketrain to composition. Either way we will have to fundamentally change a few classes. This would be greatly simplified by having them all derived from a single base class.

As for reimplementing the array interface, this wouldn't be a hard requirement. We could, for example, just implement slicing, and then let the user do any array mathematics they want directly on the composited array or arrays. It would also make the operations more explicit, since users will always know exactly what mathematical operations are being done. The downside is that there is an additional attribute that will always have to be included by users.

[rproepp]

When I argued against inheritance a year ago, my main argument was to enable transparent lazy (deferred) loading. With lazy cascading, we now have a way to do this for whole objects even when sticking to inheritance.

We still cannot do partial loading of arrays, e.g. with the h5py dataset. However, as Todd mentioned in #133, that would require using another package for handling physical quantities, as well. And then this would still only apply to HDF5; we would have to write our own proxy array to support other formats.

So I composition would be more explicit and possibly more future-proof, but inheritance is more convenient and would not change the API. I would now vote for keeping inheritance, at least if we do not switch the quantities package.

[toddrjen]

So it sounds like this depends on resolving #133. We need to make a decision there before we can decide on what to do here.

[samuelgarcia]

I think the debate is closed and inheritenace wins.