Skip to content
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

[Feature] check_consistency_of_spiketrains as utility function #326

Closed
morales-gregorio opened this issue May 20, 2020 · 2 comments
Closed

Comments

@morales-gregorio
Copy link
Collaborator

Feature
Move the _check_consistency_of_spiketrainlist() from elephant.statistics to utils or somewhere where it can be accessed by all modules.
Also make this function check for a list of length 0.

Motivation
@Kleinjohann noticed this could be helpful when working on PR #322, because we also wrote a very similar function in the new spike_train_processing module. I also noticed while working on #324 that having this input checking utility function would make the implementation of a new feature much easier with less new tests required.

Additional context
Actually this would be a good moment also to go through all of the checks and collect them all into a set of functions (also for analogsignals and other inputs). It could simplify the code, since the new functions could be tested on their own and many tests would not need to specificly provide 'wrong' input to check that it raises the right error.

@dizcza
Copy link
Member

dizcza commented May 20, 2020

Feel free to modify it in either of the abovementioned pull requests.
Usually, I do it by need. It's totally OK to slightly modify unrelated modules in a PR just to generalize the code.

Actually this would be a good moment also to go through all of the checks and collect them all into a set of functions (also for analogsignals and other inputs).

The reason why I don't do this is that I don't want to touch the stuff I've not checked yet (most of elephant functional). I don't see the big picture, that's why I do it rather step by step.

@dizcza
Copy link
Member

dizcza commented Nov 3, 2020

It's actually moved to utils in one of the pull requests that I don't recall.
I find it indeed useful.

@dizcza dizcza closed this as completed Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants