Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Deprecate alternative ways of computing inter-spike-intervals [vcs: #minor] #368

Merged
merged 40 commits into from
Feb 20, 2024

Conversation

anilbey
Copy link
Contributor

@anilbey anilbey commented Feb 8, 2024

Motivation

Having 3 different implementations for computing the inter-spike-intervals (a commonly used feature) is causing lots of inconsistencies. This PR reduces those three to only a single implementation.

Changes

  • Reduce 3 alternative implementations to get ISIs into 1.
  • "all_ISI_values" is recommended, "ISI_values" and "ISIs" are deprecated.
  • The features depending on "LibV1:ISI_values" are moved to Python and now they depend on "all_ISI_values".
  • BUGFIX: single_burst_ratio, irregularity_index, burst_mean_freq, interburst_voltage features were ignoring the first two ISIs instead of one when the ignore_first_ISI was set.
  • Added new feature: inv_ISI_values that computes and returns all of the inverse isi values. Feature documentation? #208
  • Drop python3.8 from the tox and gh-actions since scipy dependency's new version supports between py39-312.
    The users with python3.8 can still install it manually (the setup.py does not prevent it).

Checklist:

  • [ X ] Unit tests are added to cover the changes.
  • [ X ] The changes are mentioned in the documentation
  • [ X ] CHANGELOG.md is updated (by default the change will increment the patch version, to increment major or minor changes add #major or #minor to the PR description)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (1c02c12) 66.22% compared to head (53cbea3) 68.49%.

Files Patch % Lines
efel/pyfeatures/isi.py 97.43% 6 Missing ⚠️
efel/cppcore/Utils.cpp 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   66.22%   68.49%   +2.27%     
==========================================
  Files          33       36       +3     
  Lines        6504     6446      -58     
  Branches     2279     2087     -192     
==========================================
+ Hits         4307     4415     +108     
- Misses        273      275       +2     
+ Partials     1924     1756     -168     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anilbey anilbey marked this pull request as ready for review February 8, 2024 09:35
Copy link
Collaborator

@ilkilic ilkilic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! looks good to me

@anilbey anilbey merged commit bffef33 into master Feb 20, 2024
8 checks passed
@anilbey anilbey deleted the isi branch February 20, 2024 15:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants