Skip to content

Update spike time calculations for scattered non-spiking units#1871

Open
CodyCBakerPhD wants to merge 6 commits into
masterfrom
fix-more-nonspiking
Open

Update spike time calculations for scattered non-spiking units#1871
CodyCBakerPhD wants to merge 6 commits into
masterfrom
fix-more-nonspiking

Conversation

@CodyCBakerPhD
Copy link
Copy Markdown
Contributor

@CodyCBakerPhD CodyCBakerPhD commented Jun 2, 2026

Update session duration spike-time boundary handling to correctly support multiple non-spiking units scattered throughout a Units table, not just when the first unit is non-spiking.

The implementation now keeps only unit boundaries where cumulative spike count increases, which avoids invalid indexing from repeated boundaries contributed by non-spiking rows while preserving correct min/max spike-time extraction.

Also adds a regression test covering interleaved spiking and non-spiking unit rows to validate the expected session duration behavior.

…ther non-spiking units

Fix bug in spike time handling by using unique indices.
@CodyCBakerPhD CodyCBakerPhD self-assigned this Jun 2, 2026
@CodyCBakerPhD CodyCBakerPhD changed the title Update spike time calculations to use unique indices to account for o… Update spike time calculations Jun 2, 2026
@CodyCBakerPhD CodyCBakerPhD added bug Something isn't working patch Increment the patch version when merged labels Jun 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.95%. Comparing base (98a87d4) to head (40664fb).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1871      +/-   ##
==========================================
+ Coverage   52.94%   52.95%   +0.01%     
==========================================
  Files          87       87              
  Lines       12788    12787       -1     
==========================================
+ Hits         6770     6771       +1     
+ Misses       6018     6016       -2     
Flag Coverage Δ
unittests 52.95% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@CodyCBakerPhD
Copy link
Copy Markdown
Contributor Author

@copilot Please add one new test for this case (of having multiple non-spiking units scattered throughout the rows of a untis table)

Copilot AI and others added 5 commits June 2, 2026 04:18
Removed unused imports for metadata extraction.
Removed unnecessary import statements for get_metadata in multiple test cases.
Remove unnecessary blank line before assertions.
Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 2, 2026

@copilot Please add one new test for this case (of having multiple non-spiking units scattered throughout the rows of a untis table)

Added in commit 9acfc582: a new regression test (test_session_duration_with_scattered_non_spiking_units) covering multiple non-spiking units scattered throughout the Units table, plus the corresponding spike-index handling fix in _get_session_duration.

Copilot AI changed the title Update spike time calculations Update spike time calculations for scattered non-spiking units Jun 2, 2026
@CodyCBakerPhD CodyCBakerPhD moved this to Uncategorized in My Work History Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants