Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions neo/rawio/axonrawio.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,22 @@ def _parse_header(self):
signal_channels = []
adc_nums = []
for chan_index, chan_id in enumerate(channel_ids):
# Name fields are fixed-width and right-padded with spaces (v1) or already trimmed (v2).
# Strip the padding but keep interior spaces (e.g. "IN 1"); errors="replace" so an odd
# byte can never crash the read.
if version < 2.0:
name = info["sADCChannelName"][chan_id].replace(b" ", b"")
name = info["sADCChannelName"][chan_id].decode("utf-8", errors="replace").strip()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One last question. I remember on the spikeinterface side we had some formats that failed with 'utf-8' decoding. Is there a failure possibility here using this type of decode? Or the replace guarantees this will always work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So, errors="replace" avoids the raise. Instead of throwing UnicodeDecodeError on invalid bytes, it substitutes them with the replacement char, so the decode can never crash the read no matter what's in the field. The strict default (decode("utf-8") with no errors=) would raise.

What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm just wondering if raising an error is important in this case for the regular end-user? If this can't be UTF-8 is that a signifier that should not be silently glossed over? Again I don't use this format at all, so I can't say if replace or not is the better option.

I think from your perspective you'd prefer not to crash so you can inspect the file and decide for yourself whether it is okay or not. But for the non-power user is there a benefit to not even loading poorly formed formed channel names?

@h-mayorquin h-mayorquin Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think it is better that they get ugly channel names rather than not being able to load their data. But I am not sure this will ever happen. This is kind of edging against axon encoding badly for some reason.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay sounds good to me.

units = safe_decode_units(info["sADCUnits"][chan_id])
adc_num = info["nADCPtoLChannelMap"][chan_id]
elif version >= 2.0:
ADCInfo = info["listADCInfo"][chan_id]
name = ADCInfo["ADCChNames"].replace(b" ", b"")
name = ADCInfo["ADCChNames"].decode("utf-8", errors="replace").strip()
units = safe_decode_units(ADCInfo["ADCChUnits"])
adc_num = ADCInfo["nADCNum"]
if not name:
# A blank name leaves the channel unaddressable; fall back to a positional name so
# every channel keeps a usable id.
name = f"ch{chan_id}"
adc_nums.append(adc_num)

if info["nDataFormat"] == 0:
Expand Down Expand Up @@ -705,9 +712,10 @@ def _parse_abf_v1(f, header_description):
listTag.append(tag)
header["listTag"] = listTag

# protocol name formatting
header["sProtocolPath"] = clean_string(header["sProtocolPath"])
header["sProtocolPath"] = header["sProtocolPath"].replace(b"\\", b"/")
# protocol name formatting. Decode to str (like the channel names) so consumers get a plain
# string rather than a bytes value, whose str() would bake the "b'...'" repr into the path.
header["sProtocolPath"] = clean_string(header["sProtocolPath"]).decode("utf-8", errors="replace")
header["sProtocolPath"] = header["sProtocolPath"].replace("\\", "/")

# date and time
# A "no date" sentinel means there is no date to build, so fall back to rec_datetime=None. The
Expand Down Expand Up @@ -1012,7 +1020,8 @@ def _parse_abf_v2(f, header_description):
else:
protocol[key] = np.array(val)
header["protocol"] = protocol
header["sProtocolPath"] = strings[header["uProtocolPathIndex"]]
# Decode to str (like the channel names) so consumers get a plain string, not raw bytes.
header["sProtocolPath"] = strings[header["uProtocolPathIndex"]].decode("utf-8", errors="replace")

# tags
listTag = []
Expand Down
27 changes: 27 additions & 0 deletions neo/test/rawiotest/test_axonrawio.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,33 @@ def test_read_raw_protocol(self):

reader.read_raw_protocol()

def test_empty_channel_name_gets_fallback(self):
# Some ABF files store a blank ADC channel name, which collapses to "" after space
# stripping and leaves the channel unaddressable by name. A positional fallback (ch{id})
# must be used instead so every channel keeps a usable name.
path = self.get_local_path("axon/intracellular_data/abf1_episodic_empty_channel_name.abf")
reader = AxonRawIO(filename=path)
reader.parse_header()
names = list(reader.header["signal_channels"]["name"])
self.assertNotIn("", names)
self.assertEqual(names, ["ch0"])

def test_channel_name_keeps_interior_space(self):
# Channel names are stripped of padding but keep interior spaces (e.g. "IN 1", not "IN1")
# and are returned as str.
reader = AxonRawIO(filename=self.get_local_path("axon/File_axon_7.abf"))
reader.parse_header()
names = list(reader.header["signal_channels"]["name"])
self.assertEqual(names, ["IN 1"])

def test_protocol_path_decoded_to_str(self):
# String header fields should be decoded to str, not left as raw bytes; otherwise a caller
# doing str(value) gets the "b'...'" byte-literal repr baked into the path.
for fixture in ["axon/File_axon_1.abf", "axon/File_axon_2.abf"]: # v2 and v1
reader = AxonRawIO(filename=self.get_local_path(fixture))
reader.parse_header()
self.assertIsInstance(reader._axon_info["sProtocolPath"], str)

def test_integer_overflow_size_raises(self):
# An ABF header that claims more samples than the file can hold must raise a
# clear error instead of silently returning an overflowed signal size.
Expand Down
Loading