Merge of AnalogSignal and AnalogSignalArray, etc.

[apdavison]

This is a first attempt at the next step in the API-breaking changes intended for Neo 0.4.

Changes:

• Renamed AnalogSignalArray and removed the original AnalogSignal class (cf Codejam #6 : remove scalar object #124). All signals are now represented by 2-dimensional arrays, even if there is only a single channel. For convenience, objects can still be constructed using a 1-D array.
• Matched the behaviour of IrregularlySampledSignal to that of the new AnalogSignal, i.e. it can now contain multiple channels, with all channels sampled at the same, explicitly-stored time points.
• Removed the RecordingChannel class. With most of the data types now being 2D arrays (except SpikeTrain), this class now seems superfluous. Its function is replaced by a channel_indexes attribute on RecordingChannelGroup.
• For consistency, the AnalogSignal.channel_indexes and Unit.channel_indexes attributes are now properties pointing to the channel_indexes attribute of the parent RecordingChannelGroup. This removes a redundancy in which there were two independent ways of keeping track of channels.

With these changes, the object model looks rather simple and elegant, but I'm concerned I've missed something obvious, so please check these changes don't break important use cases.

Note: I modified the tests in a rather ad hoc fashion to get them to pass, so this PR is not intended for merging yet, rather for discussion. I'll clean things up and improve the tests if the general principles are agreed upon.

Also note that of course none of the IO classes have been updated yet.

[apdavison]

@samuelgarcia @rproepp @toddrjen any comments?

[samuelgarcia]

Hi Andrew, thank you for starting this big clean.
I will read carrefully in next week.
2 fast comments:

• sure to keep RecordingChannelGroup and not change it to RecordingChannel ? I have no idea about that.
• In OpenElectrophy I lost a simple way to slip up set a of channel in a new subset of channel. It is not a big problem but I need to figure out carrefully ebout consequencies in some codes.

[apdavison]

@samuelgarcia : have you had any time to look at this again?

Concerning the name "RecordingChannelGroup", we could change it to "RecordingChannelIndex". I think that might be a more accurate name.

[mdenker]

I am very happy about the change with AnalogSignal, this is also very much in line with what users in our lab would prefer.

Regarding the change in the RecordingChannel, in principle I agree that this could be a step forward. I frequently notice puzzled looks when trying to explain the hierarchy of RecordingChannel and RecordingChannelGroup to others (although in principle, it was pretty clear as such). In addition, Andrew is right that the channel_index had always caused confusion due to its ambiguity with the hierarchy.

I find that it is difficult to foresee the problems that might arise due to this change, and I could not come up with any particular scenario that might be super problematic. I think that almost any recording (even simulation) will be able to distribute a meaningful index number per "channel", so that creating a fitting data structure to match up almost any data set should be straightforward.

What is important in my opinion is to provide (by and by) an API for all common use-cases to traverse the hierarchy and the array-like data structures by channel_index. For instance, what are the AnalogSignals that belong to my SpikeTrain/Unit. Furthermore, I guess it would be good to have Neo check that the tree hierarchy is always consistent with respect to channel indices (e.g., duplicate channel indices, etc...), and that Neo provides functions such as "get_all_channel_indices()" for easy access.

In short I support to give this simplification of the structure a try.

In terms of the name, I like "RecrodingChannelIndex" because it avoids confusion with the existing object. But how about shortening this object to "ChannelIndex"? I would image that in ephys, the concept of "channel" is pretty clear even without attaching "Recording".

[apdavison]

I like the idea of "ChannelIndex"

[apdavison]

@samuelgarcia @mdenker I've now adapted most of the IO classes, at least partially. The biggest changes are to BlackrockIO - it now puts signals into a pair of 2D AnalogSignals rather than into a list of 1D AnalogSignals, but I only had the one test file to look at, so I'm not sure how general it is. Please take a look at all of the changes, but in particular Michael could you test the updated BlackrockIO with a wider selection of files?

[samuelgarcia]

Hi Andrew,
sorry to not be reactive on this task. Thank you doing this hard job.
Before merging this would it be possible to make a release neo 0.3.4 from the master ?

I don't anderstand your comment on BlackrockIO. I don't see 2D analogSignal in BlackrockIO.read_nsx but still a list of 1D AnalogSignal. Did you need some help to do this ?

[apdavison]

I don't anderstand your comment on BlackrockIO. I don't see 2D analogSignal in BlackrockIO.read_nsx but still a list of 1D AnalogSignal

See this line: https://github.com/apdavison/python-neo/blob/analogsignal/neo/io/blackrockio.py#L398

channel_filter is an array, which picks out all the channels with id < 129. Therefore in general sig will be a 2D array.

[apdavison]

Before merging this would it be possible to make a release neo 0.3.4 from the master ?

This PR is anyway for merging into the apibreak branch, not into master. We should definitely make an 0.3.4 release before we merge apibreak into master, but before then I want to improve the documentation, add a "migrating from 0.3 to 0.4" guide, and fix the HDF5IO.

[samuelgarcia]

OK thanks I have not seen it.

[apdavison]

I'm going to merge this now, and then finish the work in the apibreak branch.