Combine peakdet.Physio and phys2bids.BlueprintOutput into new base class

[tsalo]

Create a new physiopy base class for physio data with the strengths of both phys2bids.BlueprintOutput and peakdet.Physio.

[tsalo]

Elements worth retaining in combined class:

• BlueprintOutput:
  • Stores multiple channels with the same sample-rate
  • A couple of reformatting methods
• Physio:
  • Stores history of changes to data
  • Is compatible with peakdet functions.

[tsalo]

@smoia is it critical that this new base class support multiple time series or can it be modality-specific?

[tsalo]

Other desiderata:

• Ability to read from BIDS-format physio files.
• get_associated() for other physio files from same scan and for associated imaging data.
• Ability to save to BIDS Derivatives-format file.

[smoia]

@tsalo what do you mean by modality specific?
Technically this should be the class that we bring around our suite, independently from the content.
IMMO it should support multiple timeseries, mainly because we could virtually import everything in some packages like phys2denoise. Could you specify a bit more what do you mean by "modality specific" though? Maybe I don't understand...

For other desiderata:

1. Definitely
2. Good idea
3. Yes!

[tsalo]

Physio currently allows one-dimensional time series, so you create on instance of the class for each channel you want to process. On the other hand, BlueprintOutput seems to store all channels with the same sampling frequency together. So, in the case of three channels (let's say ECG, EDA, and respiration), where two channels have the same frequency and one has another frequency, with BlueprintOutput you'd need two variables, while with Physio you'd need three.

It definitely makes the denoising functions simpler to just split by channel/modality, but it makes the I/O steps simpler to keep them together. I wanted to check with you about it before committing to one method or the other.

[smoia]

Ooooh I see what you mean now.
I would tend toward the BlueprintOutput structure - I might be biased, but it's because that class should be mirroring a BIDS physio file.
In the example you're giving, we would have two bids files, so I'd work with each one independently. I also don't see why packages like phys2denoise shouldn't be able to work with all the channels together - if we want RETRICOR, we need both respiratory and cardiac functions.

Denoising is simpler in single instances though, you're right. One possibility is that we actually don't touch peakdet - it's working and doing what it should be doing.

[tsalo]

The more I work on it, the more I think keeping them separate might be the way to go. Here's what I'm thinking:

• BlueprintOutput --> BIDSPhysioFile (not to step on pybids' toes)

  • Methods:
    • to_file(filename): save BIDS files (tsv.gz + json).
    • get_channel(): select one channel and return a Physio object (peakdet's class with some updates)
  • Attributes:
    • data: 2D array with channel-wise time series
    • fs: Sampling frequency (constant across channels)
    • start_time: Start time (in seconds, from BIDS specification)
    • channels: Names of channels, in BIDS format (e.g., cardiac, respiration, and trigger).
    • units: Units of channels.

• Physio: remains mostly the same

• Related functions:

  • physutils.read_file(): Load BIDS physio files into BIDSPhysioFile. Should grab associated json file automatically.

[tsalo]

Another possible reformulation that makes the differences more explicit:

• BlueprintInput --> MultifrequencyPhysio
  • The key difference between BlueprintInput and BlueprintOutput is that BlueprintInput supports (but doesn't require) multiple frequencies, while BlueprintOutput only supports a single frequency.
• BlueprintOutput --> SingleFrequencyPhysio
• Physio --> SingleChannelPhysio or Channel
  • Physio objects only support a single channel. In order to support multiple channels elegantly, we'd need standardized labels for different modalities, I think.

EDIT: I just noticed physiopy/prep4phys#19, so maybe it would be worth it to pursue multi-channel support in the peakdet object? Alternatively, we could just allow multiple arguments to the plotting/editing functions.

[smoia]

I like this reformulation - definitely makes things clearer outside of the scopes of phys2bids!

SingleFrequencyPhysio would be the main object we bring around across repositories (so to simplify import and coding, we could call it just Physio). We definitely want some functionalities of SingleChannelPhysio to be implemented in SingleFrequencyPhysio - e.g. history tracking (yes, please!), but it's true that, if we don't want to have interactive plots with multiple channels, we can keep SingleChannelPhysio as an independent class.

We also need to decide what roles peakdet and physutils will play in our ecosystem.
Mainly, we need to decide whether some functionalities of peakdet (I'm mainly thinking about interactive plots) should be an independent package, or if that repo should contain all the necessary objects that we want across packages (effectively superseding physutils).
In the first case, it would make more sense to maintain the three classes (knowing that MultifrequencyPhysio will be in phys2bids only). In the second case, the difference between SingleFrequencyPhysio and SingleChannelPhysio is probably less meaningful - although we can start with two separate classes and eventually merge them at a later moment.
I personally favour the second option, tbh.

I'd like to discuss this issue with the other devs too. @physiopy/all, please have a look at this issue and parts of code referenced here for the meeting!

[smoia]

Another thing we might want to discuss is if we want to adopt Neurokit as a dependency - in that case we might have to map the overlap between its functions and what we already have around, as well as what we need.