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)