Skip to content

Conversation

@apdavison
Copy link
Member

@apdavison apdavison commented Jun 11, 2021

Please note that the internal implementation can probably be improved/optimised, but that is likely to need some profiling.

The general idea is to create SpikeTrain objects only when necessary, and to provide the "multiplexed" property to allow treating spike trains directly as (spike_time_array, channel_id_array)

@pep8speaks
Copy link

pep8speaks commented Jun 11, 2021

Hello @apdavison! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 34:100: E501 line too long (104 > 99 characters)
Line 314:25: E125 continuation line with same indent as next logical line

Comment last updated at 2021-06-28 09:51:52 UTC

@apdavison
Copy link
Member Author

tests all passing now

@JuliaSprenger
Copy link
Member

@apdavison Thanks for the new feature worthy of the 1000th PR / issue :)
I think SpikeTrainLists can be very usefull when dealing with multiplexed data, however with the current implementation any full neo structure (including segments & groups) will only use the original item-based SpikeTrain representation, as both parent objects contain links to different spiketrains. Is this correct? I am not sure if we can improve this somehow to still benefit from the multiplexing also in this scenario. Do you have any ideas for this?

@apdavison
Copy link
Member Author

@JuliaSprenger that's a good point. We could perhaps generalise ChannelView to index specific spike trains within the SpikeTrainList.

@apdavison
Copy link
Member Author

if the tests all pass, this is ready to merge, I think

and fix some typos in documentation
@JuliaSprenger JuliaSprenger merged commit 47a5aad into NeuralEnsemble:master Jun 28, 2021
@apdavison apdavison deleted the spiketrainlist branch June 28, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants