SignalMixin.wr_dats: acknowledge that optional fields aren't checked.

Benjamin Moody · Benjamin Moody · commit cac2af624213 · 2022-09-22T16:54:55.000-04:00
The original intention of this code was that get_write_fields would
return a list of signal information fields to be saved, and
check_sig_cohesion would check that those specified fields matched the
actual signal data.

However, this did not do what was intended.
HeaderMixin.get_write_fields returns a 2-tuple of (rec_write_fields,
sig_write_fields), not a list of field names.  Therefore, the
parameter to check_sig_cohesion never had "checksum" or "init_value"
as elements, and therefore, check_sig_cohesion never actually checked
the values of these fields when writing a record.

Many existing test cases rely on being able to read and write a record
as-is (missing fields, incorrect init_values, checksums that are
correct mod 2**16 but not equal to the result of calc_checksum); these
tests would fail if Record.wrsamp actually required init_value and
checksum fields to match as check_sig_cohesion expects.

This code should be redesigned, and the test cases probably should be
less strict.  In the meantime, however, make it clear that this
function isn't actually doing what it was pretending to do.
diff --git a/wfdb/io/_signal.py b/wfdb/io/_signal.py
@@ -142,7 +142,7 @@ def wr_dats(self, expanded, write_dir):
         # Get all the fields used to write the header
         # Assuming this method was called through wrsamp,
         # these will have already been checked in wrheader()
-        write_fields = self.get_write_fields()
+        _, _ = self.get_write_fields()
 
         if expanded:
             # Using list of arrays e_d_signal
@@ -152,8 +152,10 @@ def wr_dats(self, expanded, write_dir):
             self.check_field("d_signal")
 
         # Check the cohesion of the d_signal field against the other
-        # fields used to write the header
-        self.check_sig_cohesion(write_fields, expanded)
+        # fields used to write the header.  (Note that for historical
+        # reasons, this doesn't actually check any of the optional
+        # header fields.)
+        self.check_sig_cohesion([], expanded)
 
         # Write each of the specified dat files
         self.wr_dat_files(expanded=expanded, write_dir=write_dir)
