Skip to content

Fix Neuralynx unclosed recording header parsing - #1902

Open
Yi-111-a wants to merge 3 commits into
NeuralEnsemble:masterfrom
Yi-111-a:fix-neuralynx-unclosed-date
Open

Yi-111-a wants to merge 3 commits into
NeuralEnsemble:masterfrom
Yi-111-a:fix-neuralynx-unclosed-date

Conversation

@Yi-111-a

Copy link
Copy Markdown

Closes #1901

Summary

  • tolerate Neuralynx close headers containing the explicit File was not properly closed sentinel
  • leave recording_closed unavailable instead of passing the sentinel to dateutil.parser
  • add a focused regression test for the malformed close line

Testing

  • python -m pytest -q neo/test/rawiotest/test_neuralynxrawio.py::TestNlxHeaderParsing::test_unclosed_recording
  • python -m black --check neo/rawio/neuralynxrawio/nlxheader.py neo/test/rawiotest/test_neuralynxrawio.py

@zm711

zm711 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

This looks fine to me. Let's make sure tests pass and then happy to merge :) Thanks for the contribution.

@zm711

zm711 commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Failure is currently related to our data repository having some flakiness issues. We will try to get that sorted to ensure this is working with IO tests.

@Yi-111-a

Yi-111-a commented Oct 4, 2026

Copy link
Copy Markdown
Author

@zm711 Thanks — noted on the data-repo flake. Closing #1903 as a duplicate of this one (same #1901 goal; this PR is the sentinel-specific fix you already looked at). Happy to wait on the IO tests.

@alejoe91 alejoe91 added this to the 0.14.6 milestone Oct 5, 2026
@alejoe91

alejoe91 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

@zm711 @h-mayorquin ok to merge this and close #1908 ?

@zm711

zm711 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

We wanted one update. In #1908 Heberto added a warning. Could we have @Yi-111-a add the warning from 1908 in this one and then we will merge this! Thanks for pinging Alessio!

…lynx recording

Folds the warning from NeuralEnsemble#1908 into this branch, so the unclosed-recording case
both leaves `recording_closed` unset and tells the user why, instead of
parsing the sentinel text through dateutil.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Yi-111-a

Yi-111-a commented Oct 9, 2026

Copy link
Copy Markdown
Author

Done — pushed b2769e5, adding Heberto's warning from #1908 on top of the existing guard:

if sr:
    dt2 = sr.groupdict()
    if (dt2["date"], dt2["time"]) == ("File", "was"):
        # Cheetah writes this in place of the close time, see #1901
        warnings.warn("Text header does not contain recording closed time. File was not closed properly.")
    else:
        self["recording_closed"] = dateutil.parser.parse(f"{dt2['date']} {dt2['time']}")

The test now wraps the call in assertWarnsRegex and still asserts recording_closed stays unset, so both halves are covered. Verified locally that a properly closed header still parses to 2000-01-01 20:00:00 with no warning emitted — the change only affects the sentinel case.

Happy for this to supersede #1908 — I'll leave that one to @HectorBC and close it if you prefer, since it's your call which of the two lands first.

Re the IO tests: understood, no action needed from me on the data-repo flakiness — the parsing itself no longer routes the sentinel string through dateutil, which is where the ValueError used to come from.

@zm711

zm711 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

We'll merge this one. I talked to Heberto, so we will close #1908 and keep this one. Let me update and hope that the tests run which would be nice. But we can also get @h-mayorquin to maybe run a test data set against this if he has anything since he has worked on Neuralynx most recently.

This branch has not been deployed

No deployments
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.

Neuralynx DateTime incorrectly parsed

3 participants