[Feature] Merging "Total spiking probability edges" into elephant

[zottelsheep]

Summary:
TSPE enables the classification of excitatory and inhibitory synaptic effects using a connectivity estimation. It uses the cross-correlation of spiketrains at different delays combined with specially designed Edge-Filters to distinguish between excitatory and inhibitory connections.

Related issue: #557

Currently implemented:

• generate_filter_pairs
• normalized_cross_correlation
• total_spiking_probability_edges
• get_connectivity_matrix
• normalization function inside tspe
  Not sure if working correctly.

Current tests implemented for:

• generate_filter_pairs (Fairly Basic Test)
• normalized_cross_correlation (Fairly Basic Test)
• total_spiking_probability_edges
  The test is implemented, but currently not pushed because I need help where to put the evaluation data

What remains to be done:

• Upload evaluation data to data-repository
• Validate test-cases
• Proper documentation for:
  • total_spiking_probability_edges
  • Module documentation?
• Add reference to the Elephant bibliography file

[coveralls]

coverage: 87.024% (-1.0%) from 87.996% when pulling 6a1e08d on zottelsheep:total_spiking_probability_edges into a8aab43 on NeuralEnsemble:master.

[zottelsheep]

Hey 👋
I'm back on working on this again. Would it be possible for you @Moritz-Alexander-Kern to give me some feedback regarding the current implementation and help me getting the test-data into the data-repositories?
Then I can merge and finish the test for TSPE.

I hope I'll hear from you soon :)

[Moritz-Alexander-Kern]

Hey @zottelsheep

It's great to hear that you're making progress on this 🙂 .

I would be more than happy to assist you with the data part. Can you please let me know where I can find the data ? Once I have that, I can help you get the test-data into the repositories, so you can finish the TSPE test and merge the changes.

Is this the relevant data? (zottelsheep /tspe-python/reference): https://github.com/zottelsheep/tspe-python/tree/main/reference

Alternatively you can:

• sign up on https://gin.g-node.org/
• create a fork of the NeuralEnsemble/elephant-data repo https://gin.g-node.org/NeuralEnsemble/elephant-data .
• add the relevant data to elephant-data/unittest/functional_connectivity/total_spiking_probability_edges/data
• add a README.md , LICENSE.md , and if required a scripts folder
• you can check the tutorials or unittest folders for reference
• grant me access to the created fork and I can transfer this to NeuralEnsemble/elephant-data

I already started reviewing the current implementation, I will deliver the the review soon.

[zottelsheep]

Perfekt 👍

Yes, all relevant data is in the folder reference/evaluation_data. I have added a basic Readme and License specifically for the reference-folder. The scripts are not necessary, since these only contain the original MATALAB-Implementation and are nor used for evaluating the python-implementation.

[Moritz-Alexander-Kern]

Hello @zottelsheep,

Great job so far! I have some minor comments regarding code style conventions and naming. It would be best to follow a line length of 79 characters per line to improve readability.

Once you finished the implementation, feel free to also add yourself to the list of authors, here: doc/authors.rst and the .zenodo.json with your orcid.

[zottelsheep]

Thanks for the review @Moritz-Alexander-Kern !
I've committed the changes and added further documentation.

I would also like to commit the test-code for tspe, but I still don't know how to access the data-repository from within pytest. Is there a guide somewhere? I didn't find anything in the contribution guidelines.

[zottelsheep]

Found a way using elephant.datasets.download_datasets.
Now all that remains is to upload the data into the data-repository to the path unittests/total_spiking_probability_edges/data. Would it be ok for you @Moritz-Alexander-Kern to do that?

[Moritz-Alexander-Kern]

Hi @zottelsheep ,

Thank you for your patience, it's great to hear that you've committed the changes and added further documentation.

Regarding your question about accessing the data-repository, you found the the way I would have recommended to download the datasets using ´elephant.datasets.download_datasets´. 💯

As for uploading the data into the data-repository: I created a PR on elephant-data, see:
https://gin.g-node.org/NeuralEnsemble/elephant-data/pulls/3

Before we can proceed with merging the PR, it's important to confirm that the data is correct.

Could you please make sure it's accurate and complete (specifically README.md and LICENSE), and let me know in case any changes are required?
See: https://gin.g-node.org/NeuralEnsemble/elephant-data/pulls/3/files

[zottelsheep]

Before we can proceed with merging the PR, it's important to confirm that the data is correct.

Could you please make sure it's accurate and complete (specifically README.md and LICENSE), and let me know in case any changes are required? See: https://gin.g-node.org/NeuralEnsemble/elephant-data/pulls/3/files

LGTM!

[Moritz-Alexander-Kern]

Hey @zottelsheep ,

Thank you for this nice contribution to Elephant.

I would like to include your ORCID to properly credit your work and ensure that your contributions are recognized in the next Zenodo release, see also: .zenodo.json.

Is this your ORCID: https://orcid.org/0009-0003-9352-9826 ?

I will come back soon with the results of the review. If you have any further questions, please don't hesitate to reach out.

[Moritz-Alexander-Kern]

Hey @zottelsheep ,

Here is an initial review focusing on code style and other formal aspects. Kindly grant me some additional time to thoroughly examine the code for a comprehensive scientific review of the new functionality.

Thanks again for this contribution.

[Moritz-Alexander-Kern]

Hey @zottelsheep ,

I fixed the pep8 issues and converted the tests using the unitest frame.

You can merge those changes, by approving this PR: zottelsheep#1

[Moritz-Alexander-Kern]

Thanks for merging @zottelsheep 🎉 .

Ready to be merged into elephant from my side.

[Moritz-Alexander-Kern]

Build wheels.

[zottelsheep]

I approved and merged the PR. Thank you very much @Moritz-Alexander-Kern!

I've been rather busy with my bachelorthesis at the moment, and haven't had much time yet to look at the state of the PR.

LGTM as well!

[Moritz-Alexander-Kern]

I approved and merged the PR. Thank you very much @Moritz-Alexander-Kern!

I've been rather busy with my bachelorthesis at the moment, and haven't had much time yet to look at the state of the PR.

LGTM as well!

No worries, thanks for your work and all the best for your thesis 🚀

[zottelsheep]

Ready to be merged into elephant from my side.

Send it 🚀 😃

[zottelsheep]

Hey @Moritz-Alexander-Kern,
is there a reasons this isn't merged yet?

[Moritz-Alexander-Kern]

Hey @zottelsheep,

Thanks for your patience and for reaching out about the status of your pull request. The delay in merging it; was because we were deeply engaged in working on the release of version 1.0.0, a major milestone for our project.

This PR is scheduled for the upcoming release. Additionally, we'd like to properly credit your work by adding your ORCID to ensure recognition in the next Zenodo release.

Is this your correct ORCID: https://orcid.org/0009-0003-9352-9826?

[pep8speaks]

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

• In the file elephant/functional_connectivity.py:

Line 2:80: E501 line too long (85 > 79 characters)
Line 3:80: E501 line too long (81 > 79 characters)
Line 26:80: E501 line too long (82 > 79 characters)
Line 31:1: W391 blank line at end of file

• In the file elephant/test/test_total_spiking_probability_edges.py:

Line 69:63: E231 missing whitespace after ','

Comment last updated at 2024-02-23 15:55:00 UTC

[zottelsheep]

Great to hear! Yes that is my ORCID. I've included my information in the .zenodo.json. Thanks!