Fix performance issue for aggregate_channels

[alejoe91]

Fixes #2625

[zm711]

Looks good to me. Just a couple quick questions, but if you want to ignore them let me know and I can just approve it and we can think about them later. See my comment about the channel_indices=None.

My second question is in the else statement we iterate through all the segments (L158), but if we are using the recording_id above wouldn't it make sense that even if the user supplies channels we make sure those channels are in that segment or does that not generate a big enough slowdown to check?

The actual changes in this PR make sense though. We just collect up all the channels we need and make one pass rather than go channel-by-channel.

[alejoe91]

My second question is in the else statement we iterate through all the segments (L158), but if we are using the recording_id above wouldn't it make sense that even if the user supplies channels we make sure those channels are in that segment or does that not generate a big enough slowdown to check?

Yes this can be a bit misleading! In the else statement we just grab all channels because we check previously that we need to return all of them in the correct order, so no need to use the mapping.

[zm711]

Cool. Looks good to me then.