-
Notifications
You must be signed in to change notification settings - Fork 266
include analog signal merging utility function and the corresponding … #846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fc04ac7
6e2c239
c1520f3
cea2115
7876e5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,86 @@ | |
| import quantities as pq | ||
|
|
||
|
|
||
| def merge_anasiglist(anasiglist, axis=1): | ||
| """ | ||
| Merges neo.AnalogSignal objects into a single object. | ||
| Units, sampling_rate, t_start, t_stop and signals shape must be the same | ||
| for all signals. Otherwise a ValueError is raised. | ||
| Parameters | ||
| ---------- | ||
| anasiglist: list of neo.AnalogSignal | ||
| list of analogsignals that will be merged | ||
| axis: int | ||
| axis along which to perform the merging | ||
| `axis = 1` corresponds to stacking the analogsignals | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By implementing the combination of signals along two different axis you are covering two different scenarios, that require different checks on the neo level. Eg. when stacking signals (ie merging multiple channels in one Analogsignal) it is useful to ensure they have the same length and cover the same time range. However for concatenating analogsignals along time, these conditions do not need to be fulfilled. I would argue therefore these different cases should also be implemented in separate functions. This would also make it easier for the users to decide which of the actions they actually want to use.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another feature that needs to be treated differently depending on the axis are the array_annotations. I am not sure in how far this implementation is taking care of this, but I think we should double check here. |
||
| `axis = 0` corresponds to concatenating the analogsignals | ||
| Default: 1 | ||
| Returns | ||
| ------- | ||
| merged_anasig: neo.AnalogSignal | ||
| merged output signal | ||
| """ | ||
|
|
||
| # Sanity of inputs | ||
| if not isinstance(anasiglist, list): | ||
| raise TypeError('anasiglist must be a list') | ||
| if not isinstance(axis, int) or axis not in [0, 1]: | ||
| raise TypeError('axis must be 0 or 1') | ||
| if len(anasiglist) == 0: | ||
| raise ValueError('Empty list! nothing to be merged') | ||
| if len(anasiglist) == 1: | ||
| raise ValueError('Passed list of length 1, nothing to be merged') | ||
|
|
||
| # Check units, sampling_rate, t_start, t_stop and signal shape | ||
| for anasig in anasiglist: | ||
| if not anasiglist[0].units == anasig.units: | ||
| raise ValueError('Units must be the same for all signals') | ||
| if not anasiglist[0].sampling_rate == anasig.sampling_rate: | ||
| raise ValueError('Sampling rate must be the same for all signals') | ||
| if not anasiglist[0].t_start == anasig.t_start: | ||
| raise ValueError('t_start must be the same for all signals') | ||
| if axis == 0: | ||
| if not anasiglist[0].magnitude.shape[1] == \ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The number of channels is represented by the last dimension, so better check |
||
| anasig.magnitude.shape[1]: | ||
| raise ValueError('Analogsignals appear to contain different ' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ...then you can also remove the |
||
| 'number of channels!') | ||
| if axis == 1: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should accordingly check all other dimensions except for the first one as in principle there could be more than 2. |
||
| if not anasiglist[0].magnitude.shape[0] == \ | ||
| anasig.magnitude.shape[0]: | ||
| raise ValueError('Cannot merge signals of different length.') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A reoccurring scenario is that some data can only be loaded into neo with |
||
|
|
||
| # Initialize the arrays | ||
| anasig0 = anasiglist.pop(0) | ||
| data_array = anasig0.magnitude | ||
| sr = anasig0.sampling_rate | ||
| t_start = anasig0.t_start | ||
| units = anasig0.units | ||
|
|
||
| # Get the full array annotations | ||
| for anasig in anasiglist: | ||
| anasig0.array_annotations = anasig0._merge_array_annotations(anasig) | ||
|
|
||
| array_annot = anasig0.array_annotations | ||
|
Comment on lines
+72
to
+75
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that there is a distinction missing whether the analogsignals are concatenated or stacked, which would require different handling of the array annotations.
or what do you think is a good merging strategy? |
||
| del anasig0 | ||
|
|
||
| while len(anasiglist) != 0: | ||
| anasig = anasiglist.pop(0) | ||
| data_array = np.concatenate((data_array, anasig.magnitude), | ||
| axis=axis) | ||
| del anasig | ||
|
|
||
| # Create new analogsignal object to contain the analogsignals | ||
| merged_anasig = neo.AnalogSignal(data_array, | ||
| sampling_rate=sr, | ||
| t_start=t_start, | ||
| units=units, | ||
| array_annotations=array_annot) | ||
| return merged_anasig | ||
|
|
||
|
|
||
| def get_events(container, **properties): | ||
| """ | ||
| This function returns a list of Event objects, corresponding to given | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this change is needed since Neo allows for up to 99 characters per line, see
python-neo/.pep8speaks.yml
Line 2 in 57f8d07
Maybe you are using the default PEP8 settings for autoformatting here?