From 4296ddcda3f12323a350676b9a731d4d76e53a7d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 13:13:47 +0000 Subject: [PATCH] Always bind an epoch's binary, empty or not ndi_element_timeseries.addepoch skipped writing and attaching epoch_binary_data.vhsb when timepoints or datapoints was None, and fell through to the base class, which added an element_epoch document with nothing bound to that name. MATLAB has no such branch. In ndi.element.timeseries/addepoch the vhsb_write and the add_file sit AFTER the argument handling rather than inside it, and timepoints/datapoints are required positional arguments, so the class cannot produce a fileless epoch document at all: vlt.file.custom_file_formats.vhsb_write(fname,timepoints,datapoints,...); epochdoc = epochdoc.add_file('epoch_binary_data.vhsb',fname); An epoch with NO SAMPLES is a supported case -- ndi/+mock/+fun/ subject_stimulator_neuron.m calls addepoch with [] for both, for a stimulator element that marks a time range and carries no samples -- but its binary is still written, just empty. An epoch may have no data; it may not have no file. That is exactly what element_epoch's schema says with mustbenotempty on epoch_binary_data.vhsb. DID was not checking. VH-Lab/DID-python#86 closed check_files' step-3 fail-open, which looped over the actual file_info entries and fell through to valid when a required name had none. So these documents were accepted at add time and only failed at read time; now they are refused where they are created, which is where the defect is. The two tests under TestEpochWithoutData asserted the divergence rather than the contract -- one of them was named test_addepoch_without_samples_stores_no_binary. They now hold the MATLAB behaviour: the binary is bound, it is declared on the document (bound, not merely written -- what DID rejects is a name in file_list with no matching file_info entry), and reading an epoch with no samples gives nothing rather than an error, which is what the old test was really about. Measured against main in the same working tree, with did and vhlab-toolbox-python both installed from main as CI installs them: 29 errors and 5 failures fixed, nothing newly broken. - tests/test_fun_ensemble_db.py 26 errors -> 0 - tests/test_gui_app_ensemble_maker.py 3 errors -> 0 - tests/test_element_timeseries_vhsb.py 5 failed -> 0 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YHxsZ56GZsZ3hJ98TJ81t9 --- src/ndi/element_timeseries.py | 26 +++++++++++++++++--- tests/test_element_timeseries_vhsb.py | 35 ++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/src/ndi/element_timeseries.py b/src/ndi/element_timeseries.py index 7ec5bec1..56ad7b1e 100644 --- a/src/ndi/element_timeseries.py +++ b/src/ndi/element_timeseries.py @@ -287,10 +287,28 @@ def addepoch( ``ndi.element.timeseries/addepoch``; ``add_to_database`` is the explicit form of that, since Python has no ``nargout``. """ - has_data = timepoints is not None and datapoints is not None - - if not has_data or self._session is None: - return super().addepoch(epoch_id, epoch_clock, t0_t1, add_to_database=add_to_database) + # THE BINARY IS ALWAYS WRITTEN AND ALWAYS ATTACHED, empty or not. + # + # This used to skip both when timepoints or datapoints was None and + # fall through to the base class, which added an element_epoch with + # nothing bound to epoch_binary_data.vhsb. MATLAB has no such branch: + # in ndi.element.timeseries/addepoch the vhsb_write and the add_file + # sit after the argument handling rather than inside it, and + # timepoints/datapoints are required positional arguments, so the + # class cannot produce a fileless epoch document at all. + # + # An epoch with NO SAMPLES is a supported case -- see + # ndi/+mock/+fun/subject_stimulator_neuron.m, which calls addepoch + # with [] for both -- but its binary is still written, just empty. + # An epoch may have no data; it may not have no file. That is exactly + # what element_epoch's schema says with mustbenotempty on the file, + # and DID enforces it now that DID-python#86 closed check_files' + # step-3 fail-open. Before that the invalid document was accepted at + # add time and only noticed on read. + if timepoints is None: + timepoints = np.zeros((0, 1)) + if datapoints is None: + datapoints = np.zeros((0, 1)) _, doc = super().addepoch(epoch_id, epoch_clock, t0_t1, add_to_database=False) doc = self._attach_timeseries_data(doc, timepoints, datapoints) diff --git a/tests/test_element_timeseries_vhsb.py b/tests/test_element_timeseries_vhsb.py index a3e312fd..f472f2b6 100644 --- a/tests/test_element_timeseries_vhsb.py +++ b/tests/test_element_timeseries_vhsb.py @@ -224,14 +224,41 @@ def test_mismatched_lengths_raise(self, element): class TestEpochWithoutData: - def test_addepoch_without_samples_stores_no_binary(self, element): + """An epoch may have no DATA. It may not have no FILE. + + These two used to assert the opposite -- that an epoch added without + samples stored no binary at all -- which is a document MATLAB cannot + produce. ndi.element.timeseries/addepoch writes the vhsb and attaches + it after the argument handling rather than inside it, and takes + timepoints/datapoints as required positional arguments; an epoch with + no samples goes through it as [] and [], and still gets a file, just + an empty one (ndi/+mock/+fun/subject_stimulator_neuron.m does exactly + that for a stimulator element). + + element_epoch's schema has always said so -- mustbenotempty on + epoch_binary_data.vhsb -- and DID simply was not checking until + DID-python#86 closed check_files' step-3 fail-open. So the old + assertions were pinning a Python-only divergence that produced + documents the database now rejects at add time and used to accept and + fail on at read time. + """ + + def test_addepoch_without_samples_still_binds_the_binary(self, element): session, elem = element _, doc = elem.addepoch("epoch_1", _clock(), [(0.0, 1.0)]) exists, _ = session.database_existbinarydoc(doc, BINARY_FILE_NAME) - assert not exists + assert exists, "an epoch with no samples still stores an empty vhsb" + + def test_the_bound_binary_is_declared_on_the_document(self, element): + """Bound, not merely written: what DID rejects is a name in + file_list with no matching entry in files.file_info.""" + session, elem = element + _, doc = elem.addepoch("epoch_1", _clock(), [(0.0, 1.0)]) + assert BINARY_FILE_NAME in doc.current_file_list() - def test_reading_an_epoch_with_no_binary_is_not_an_error(self, element): - """It falls through to the underlying element, which is empty here.""" + def test_reading_an_epoch_with_no_samples_gives_nothing(self, element): + """The file is there and empty, so the read is empty rather than + an error -- which is what the old test was really about.""" session, elem = element elem, _ = elem.addepoch("epoch_1", _clock(), [(0.0, 1.0)]) data, times, _ = elem.readtimeseries("epoch_1", -np.inf, np.inf)