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

Detect mismatch errors between group and group names when writing ElectrodeGroups #1165

Draft
wants to merge 2 commits into
base: fix_electrode_groups_ephys
Choose a base branch
from

Conversation

h-mayorquin
Copy link
Collaborator

This should come after #1164 and has it as its base.

This should close #932 as we have now a function that decides the right priority between "group_name" and "group". This PR adds some errors for two different types of mismatch:

  1. The group_names have different number of unique entries than groups.
  2. The group_names and the groups are consistent across the array

@alejoe91 could you take a look?

@h-mayorquin h-mayorquin requested a review from alejoe91 December 18, 2024 22:58
@h-mayorquin h-mayorquin self-assigned this Dec 18, 2024
@h-mayorquin h-mayorquin changed the title add tests to detect mismatch errors and fix it Detect mismatch errors between group and group names when writing ElectrodeGroups Dec 18, 2024
@@ -259,6 +267,23 @@ def _get_group_name(recording: BaseRecording) -> np.ndarray:
# If for any reason the group names are empty, fill them with the default
group_names[group_names == ""] = default_value

# Validate group names against groups
if groups is not None:
unique_groups = np.unique(groups)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mmm I will change this to sets as I remember np.unique being buggy for strings.

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.79%. Comparing base (c8f91c7) to head (cf579d6).

Additional details and impacted files

Impacted file tree graph

@@                      Coverage Diff                       @@
##           fix_electrode_groups_ephys    #1165      +/-   ##
==============================================================
+ Coverage                       90.75%   90.79%   +0.04%     
==============================================================
  Files                             129      129              
  Lines                            8286     8301      +15     
==============================================================
+ Hits                             7520     7537      +17     
+ Misses                            766      764       -2     
Flag Coverage Δ
unittests 90.79% <100.00%> (+0.04%) ⬆️

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

Files with missing lines Coverage Δ
...c/neuroconv/tools/spikeinterface/spikeinterface.py 91.50% <100.00%> (+0.53%) ⬆️
src/neuroconv/tools/testing/mock_interfaces.py 99.20% <100.00%> (+0.01%) ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant