Skip to content

Conversation

@h-mayorquin
Copy link
Collaborator

This has #1124 as a base and should be done after that.

This converter can be used to write sorted data and keep the correct electrode provenance of the units (display electrodes in the units table). The converted takes care of handling the correct map between units and electrodes which is important for multiple recording/probes such as the one discussed in #1112.

Also related to this:
#961

@h-mayorquin h-mayorquin self-assigned this Nov 7, 2024
@h-mayorquin h-mayorquin changed the title SortedRecordingConverter for solving unit electrode provenance SortedRecordingConverter for solving unit electrode provenance Nov 7, 2024
@h-mayorquin h-mayorquin changed the title SortedRecordingConverter for solving unit electrode provenance SortedRecordingConverter for solving unit electrode provenance in sorted data Nov 7, 2024
Base automatically changed from add_electrode_disambiguation_in_sorting_interfaces to main November 15, 2024 18:24
@h-mayorquin h-mayorquin marked this pull request as ready for review March 4, 2025 16:32
@h-mayorquin
Copy link
Collaborator Author

OK, so we can have this as a base for the manual case. In another PR coming after this we can use sortinganalyzer tools to automatically build the mapping on the fly. After that, let's think on an interface that will generalize this to converters with multiple interfaces.

@alejoe91 can you take a look at this? I will add the sorting_analyzer automatic detection in a future PR later.

@h-mayorquin h-mayorquin requested a review from alejoe91 March 4, 2025 16:35
Copy link
Contributor

@alejoe91 alejoe91 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks @h-mayorquin

@h-mayorquin h-mayorquin enabled auto-merge (squash) March 7, 2025 18:56
@h-mayorquin h-mayorquin merged commit 9d9cf7f into main Mar 7, 2025
40 checks passed
@h-mayorquin h-mayorquin deleted the add_sorted_recording branch March 7, 2025 22:00
@codecov
Copy link

codecov bot commented Jun 5, 2025

Codecov Report

Attention: Patch coverage is 70.83333% with 14 lines in your changes missing coverage. Please review.

Project coverage is 90.32%. Comparing base (9031585) to head (de77c3c).
Report is 107 commits behind head on main.

Files with missing lines Patch % Lines
...datainterfaces/ecephys/sortedrecordinginterface.py 65.85% 14 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1132      +/-   ##
==========================================
- Coverage   90.40%   90.32%   -0.09%     
==========================================
  Files         133      136       +3     
  Lines        8511     8616     +105     
==========================================
+ Hits         7694     7782      +88     
- Misses        817      834      +17     
Flag Coverage Δ
unittests 90.27% <70.83%> (-0.09%) ⬇️

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

Files with missing lines Coverage Δ
src/neuroconv/converters/__init__.py 100.00% <100.00%> (ø)
...erfaces/ecephys/baserecordingextractorinterface.py 94.00% <100.00%> (+0.18%) ⬆️
...nterfaces/ecephys/basesortingextractorinterface.py 80.76% <100.00%> (+0.57%) ⬆️
src/neuroconv/nwbconverter.py 82.26% <ø> (ø)
...datainterfaces/ecephys/sortedrecordinginterface.py 65.85% <65.85%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants