Skip to content

SortGroup.set_group_by_shank crashes on non-numeric electrode_group_name (hard-coded int() sort) #1623

Description

@pauladkisson

Describe the bug

SortGroup.set_group_by_shank(nwb_file_name=...) raises ValueError: invalid literal for int() with base 10: ... for any session whose electrodes have a non-numeric electrode_group_name.

The helper get_group_by_shank (in spikesorting/utils.py) sorts the electrode-group names with e_groups.sort(key=int), which assumes every group name is a numeric string (e.g. "0", "1"). NWB files that name electrode groups descriptively — ours use "probe1_shank1" — fail the int() cast.

This blocks the standard sort-group setup step entirely; the only workaround is to bypass set_group_by_shank and insert SortGroup / SortGroup.SortGroupElectrode rows manually.

To Reproduce

This script is self-contained: it builds a tiny NWB file with an ndx-franklab-novela Probe whose electrode group is named "probe1_shank1", inserts it into Spyglass, and then calls set_group_by_shank. (Set dj_local_conf_path to your own config.)

"""Minimal reproduction: SortGroup.set_group_by_shank crashes on a non-numeric
electrode_group_name."""
from pathlib import Path
import numpy as np
import datajoint as dj

# Point this at your own DataJoint config (or remove if already configured elsewhere).
dj_local_conf_path = "dj_local_conf.json"
dj.config.load(dj_local_conf_path)
dj.config["enable_python_native_blobs"] = True

from pynwb import NWBHDF5IO
from pynwb.testing.mock.file import mock_NWBFile, mock_Subject
from ndx_franklab_novela import Probe, Shank, ShanksElectrode, NwbElectrodeGroup

import spyglass.common as sgc
import spyglass.data_import as sgi
import spyglass.spikesorting.v1 as sgs
from spyglass.settings import raw_dir
from spyglass.utils.nwb_helper_fn import get_nwb_copy_filename


def build_nwbfile():
    nwbfile = mock_NWBFile(
        identifier="set_group_by_shank_bug",
        session_description="Mock NWB demonstrating set_group_by_shank int() bug",
    )
    mock_Subject(nwbfile=nwbfile, subject_id="set_group_by_shank_subject")

    n_channels = 4
    shanks_electrodes = [
        ShanksElectrode(name=str(i), rel_x=0.0, rel_y=0.0, rel_z=float(i))
        for i in range(n_channels)
    ]
    # Shank name must match the electrodes' probe_shank value below (Spyglass keys
    # Probe.Electrode on (probe_id, probe_shank, probe_electrode)).
    shank = Shank(name="1", shanks_electrodes=shanks_electrodes)
    # Novel probe_type / name so the example does not collide with any existing probe.
    probe = Probe(
        name="test_probe_nonnumeric_group",
        id=1,
        probe_type="test_probe_nonnumeric_group",
        probe_description="1x4 test probe",
        contact_side_numbering=True,
        contact_size=10.0,
        units="um",
        shanks=[shank],
    )
    nwbfile.add_device(probe)

    # NON-numeric electrode group name -- this is what trips set_group_by_shank.
    # Use the franklab NwbElectrodeGroup, as real franklab/novela files do.
    electrode_group = NwbElectrodeGroup(
        name="probe1_shank1",
        description="test shank",
        location="hippocampus",
        device=probe,
        targeted_location="hippocampus",
        targeted_x=0.0,
        targeted_y=0.0,
        targeted_z=0.0,
        units="um",
    )
    nwbfile.add_electrode_group(electrode_group)

    # Custom columns Spyglass expects from the franklab/novela electrode format.
    for col in ["probe_shank", "probe_electrode", "bad_channel", "ref_elect_id"]:
        nwbfile.add_electrode_column(name=col, description=col)
    for i in range(n_channels):
        nwbfile.add_electrode(
            id=i, group=electrode_group, location="hippocampus",
            probe_shank=1, probe_electrode=i, bad_channel=False, ref_elect_id=-1,
        )
    return nwbfile


def main():
    nwbfile_path = Path(raw_dir) / "set_group_by_shank_bug.nwb"
    if nwbfile_path.exists():
        nwbfile_path.unlink()
    with NWBHDF5IO(nwbfile_path, "w") as io:
        io.write(build_nwbfile())

    nwb_copy_file_name = get_nwb_copy_filename(nwbfile_path.name)
    (sgc.Nwbfile & {"nwb_file_name": nwb_copy_file_name}).delete(safemode=False)
    sgi.insert_sessions(str(nwbfile_path.name), rollback_on_fail=True, raise_err=True)

    print(
        "electrode_group_name values:",
        np.unique(
            (sgc.Electrode & {"nwb_file_name": nwb_copy_file_name}).fetch(
                "electrode_group_name"
            )
        ),
    )

    sgs.SortGroup.set_group_by_shank(nwb_file_name=nwb_copy_file_name)
    # --> ValueError: invalid literal for int() with base 10: 'probe1_shank1'


if __name__ == "__main__":
    main()

Output (the insert succeeds, then set_group_by_shank fails):

electrode_group_name values: ['probe1_shank1']
Error Stack
Traceback (most recent call last):
  File "repro_set_group_by_shank.py", line 100, in <module>
    main()
  File "repro_set_group_by_shank.py", line 95, in main
    sgs.SortGroup.set_group_by_shank(nwb_file_name=nwb_copy_file_name)
  File "/.../spyglass/src/spyglass/spikesorting/v1/recording.py", line 88, in set_group_by_shank
    sg_keys, sge_keys = get_group_by_shank(
  File "/.../spyglass/src/spyglass/spikesorting/utils.py", line 50, in get_group_by_shank
    e_groups.sort(key=int)  # sort electrode groups numerically
ValueError: invalid literal for int() with base 10: 'probe1_shank1'

Suggested fix

get_group_by_shank only needs a stable ordering of the group names; the numeric assumption is incidental. Replace the hard-coded int key with something that tolerates non-numeric names, e.g.:

try:
    e_groups.sort(key=int)
except ValueError:
    e_groups.sort()  # fall back to lexicographic order for non-numeric group names

(or a natural-sort key if numeric-aware ordering of names like "shank2" vs "shank10" matters).

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions