-
Notifications
You must be signed in to change notification settings - Fork 10
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
[New Check]: Spike times are increasing #405
base: dev
Are you sure you want to change the base?
[New Check]: Spike times are increasing #405
Conversation
Thanks @alessandratrapani. We also need units tests |
Co-authored-by: Cody Baker <[email protected]>
@alessandratrapani can you please add an entry here explaining this best practice in plain English? |
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.
lgtm!
@alessandratrapani , can you resolve the conflicts? ![]() |
|
||
|
||
@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=Units) | ||
def check_ascending_spike_times(units_table: Units): |
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.
We are still testing the performance of this, since it's the first data heavy check to be added without options to control how much data is loaded
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.
@alessandratrapani Can you add a nelems
option here to mirror other table tests?
Relate to Issue #394
Add check function for ascending spike times