Add capability to write signal with unique samps_per_frame to wfdb.io.wrsamp

[briangow]

This PR adds the capability for writing signals with unique samples per frame (samps_per_frame) to wfdb.io.wrsamp. This is typically the function that is used to write WFDB files. This was previously only possible to do by creating a Record first and using its wrsamp method to do the write.

@bemoody , I do feel like checking that the frames were uniform took a bit of extra code. I'm happy to leave it like this but also open to discussing other approaches.

I've added a couple of tests to check that this continues to work as expected.

[tompollard]

Is this intentionally changing behavior for calls where p_signal=None and d_signal=None? In the past, this would evaluate to False when e_p_signal=None and e_d_signal=None, but it now evaluates to True.

[briangow]

@tompollard , if all signals are passed as None a failure will occur further down the line. @bemoody , do we want to allow all signals to be None when calling wfdb.io.wrsamp? If we don't want to allow that then I will leave the code as I have it which requires that one of the signals is set.

[tompollard]

If passing None for both signals (p_signal and d_signal) previously resulted in an error, then the new behavior is an improvement. Unless people were intentioanlly calling the function with both signals set to None previously, this change isn't a problem.

[tompollard]

If samps_per_frame is not a list (e.g. it's an int), this will error out.

len(4)
TypeError: object of type 'int' has no len()

[briangow]

Fixed

[tompollard]

as above, len(samps_per_frame) will error if samps_per_frame is an int.

[briangow]

Fixed

[briangow]

@tompollard , if this failing test is of concern, could you guide me on what to do to resolve it:

would reformat /home/runner/work/wfdb-python/wfdb-python/wfdb/io/record.py
2 files would be reformatted, 36 files would be left unchanged.

[tompollard]

@tompollard , if this failing test is of concern, could you guide me on what to do to resolve it:

It looks like the style checks are failing (https://github.com/MIT-LCP/wfdb-python/actions/runs/11408864264/job/31747939807?pr=510). To fix, you'll need to make sure that the code conforms to black (install black, run black over the code to reformat).

[bemoody]

The following raises an error:

>>> wfdb.wrsamp("xxx", fs=500, units=["mV"], sig_name=["I"], fmt=["16"],
                e_p_signal=[numpy.array([0.0, 1.0, 2.0])],
                adc_gain=[1.0], baseline=[0])
TypeError: 'NoneType' object is not subscriptable

In this case, wrsamp should raise an exception to say that if e_p_signal is provided, then samps_per_frame must be too.

[bemoody]

The following raises an error:

>>> wfdb.wrsamp("xxx", fs=500, units=["mV"], sig_name=["I"], e_p_signal=[numpy.array([0.0, 1.0, 2.0])], fmt=["16"], samps_per_frame=[3])
TypeError: ufunc 'isinf' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

The problem here is that calc_adc_params doesn't handle the expanded case. calc_adc_params should use the same method to generate adc_gain and baseline as it does in the non-expanded case.

I think it should be straightforward to restructure calc_adc_params to take an expanded argument and to use e_p_signal instead of p_signal where appropriate.

[briangow]

The following raises an error:

>>> wfdb.wrsamp("xxx", fs=500, units=["mV"], sig_name=["I"], fmt=["16"],
                e_p_signal=[numpy.array([0.0, 1.0, 2.0])],
                adc_gain=[1.0], baseline=[0])
TypeError: 'NoneType' object is not subscriptable

In this case, wrsamp should raise an exception to say that if e_p_signal is provided, then samps_per_frame must be too.

@bemoody , thanks for catching this. I think this also applies for e_d_signal. I've added a check to make sure samps_per_frame is supplied when writing these expanded signals.

I've also updated the check to make sure samps_per_frame aligns with the channels in the signal, to consider the case when samps_per_frame is an int instead of a list.

Finally, I think we should allow passing samps_per_frame with a 1 for each channel to wrsamp when a mixed frequency signal is read in with smooth_frames=True:

rec = wfdb.io.rdrecord('/wfdb-python/sample-data/mixedsignals', smooth_frames=True)

print(rec.samps_per_frame)
[4, 4, 4, 2, 2, 1]

wfdb.io.wrsamp('written_mixedsignals', fs = rec.fs, units=rec.units, sig_name=rec.sig_name, base_date=rec.base_date, base_time=rec.base_time, comments=rec.comments, p_signal=rec.p_signal, samps_per_frame=[1,1,1,1,1,1], baseline=rec.baseline, adc_gain=rec.adc_gain, fmt=rec.fmt)

Please let me know if you disagree, or have other thoughts with respect to this case.

[briangow]

The following raises an error:

>>> wfdb.wrsamp("xxx", fs=500, units=["mV"], sig_name=["I"], e_p_signal=[numpy.array([0.0, 1.0, 2.0])], fmt=["16"], samps_per_frame=[3])
TypeError: ufunc 'isinf' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

The problem here is that calc_adc_params doesn't handle the expanded case. calc_adc_params should use the same method to generate adc_gain and baseline as it does in the non-expanded case.

I think it should be straightforward to restructure calc_adc_params to take an expanded argument and to use e_p_signal instead of p_signal where appropriate.

@bemoody , this is being addressed in a separate PR: #512 .

[bemoody]

Looks good