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

Fix bug in Intan interface where extra device is written #1059

Merged
merged 6 commits into from
Sep 10, 2024

Conversation

h-mayorquin
Copy link
Collaborator

Because the metadata in metadata["Ecephys"]["ElectrodeGroups"] contains a contains a reference to a device here:

electrode_metadata = [
dict(name=str(group_id), description="no description", location="unknown", device="DeviceEcephys")
for group_id in unique_channel_groups
]

When the code hits these lines another device is created:

for group_metadata in metadata["Ecephys"]["ElectrodeGroup"]:
if group_metadata.get("name", defaults[0]["name"]) not in nwbfile.electrode_groups:
device_name = group_metadata.get("device", defaults[0]["device"])
if device_name not in nwbfile.devices:
new_device_metadata = dict(Ecephys=dict(Device=[dict(name=device_name)]))
add_devices(nwbfile=nwbfile, metadata=new_device_metadata)
warnings.warn(
f"Device '{device_name}' not detected in "
"attempted link to electrode group! Automatically generating."
)

This resulted in me getting an nwbfile with two devices Intan and the generic one that the default metadata included.

This PR fixes this, I added a test and took the opportunity to move the Intan tests to the new format.

Plus, I made a small improvement to the metadata where the specific controller that was used is indicated as a Device Description:

https://intantech.com/RHD_system.html
https://intantech.com/RHS_system.html

Finally, I also removed a bunch of deprecations whose time has come and logic that was only valid when python-neo version requirement was lower.

@h-mayorquin h-mayorquin changed the title Fix bug in Intan Devices where two devices are written instead of two Fix bug in Intan Devices where extra device is written Sep 7, 2024
@h-mayorquin h-mayorquin changed the title Fix bug in Intan Devices where extra device is written Fix bug in Intan interface where extra device is written Sep 7, 2024
@h-mayorquin h-mayorquin marked this pull request as ready for review September 7, 2024 03:48
@h-mayorquin h-mayorquin merged commit d35b364 into main Sep 10, 2024
35 checks passed
@h-mayorquin h-mayorquin deleted the fix_intan_device branch September 10, 2024 02:10
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.31%. Comparing base (81a022d) to head (0d6c51d).
Report is 60 commits behind head on main.

Files with missing lines Patch % Lines
...c/neuroconv/tools/spikeinterface/spikeinterface.py 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1059      +/-   ##
==========================================
- Coverage   90.32%   90.31%   -0.01%     
==========================================
  Files         129      129              
  Lines        7996     7988       -8     
==========================================
- Hits         7222     7214       -8     
  Misses        774      774              
Flag Coverage Δ
unittests 90.31% <88.88%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...datainterfaces/ecephys/intan/intandatainterface.py 100.00% <100.00%> (+9.30%) ⬆️
...c/neuroconv/tools/spikeinterface/spikeinterface.py 90.77% <0.00%> (-0.93%) ⬇️

... and 1 file with indirect coverage changes

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.

2 participants