physiopy / phys2bids

Python3 library to format physiological files in BIDS. At the moment, it supports Acqknowledge and Labchart. BrainHack participants, check the issues with the BrainHack labels!
https://phys2bids.readthedocs.io
Apache License 2.0
63 stars 44 forks source link

Expanding the physiopy suite #186

Open smoia opened 4 years ago

smoia commented 4 years ago

Up until now (and presumably for the near future), phys2bids has been the primary focus of our attention, work, and development; however, when we began working on it we knew we might want to expand physiopy beyond just phys2bids. I think now, in this period of remote working and home-quarantine, might be a good opportunity to discuss what that might look like! Since data collection is currently halted for most of us, we could potentially dedicate some of this time to the development of new tools to be added to physiopy.

For instance, @CesarCaballeroGaudes proposed to create a new repository for physiological denoising, in order to prepare cardiac and respiration files for their use in RETROICOR. Some users that have not yet contributed to phys2bids offered to create utilities to improve the recordings of physiological files with Biopac. There are also some scripts I created for my PhD project to prepare CO2 traces for cerebrovascular reactivity mapping that could be packaged and released as another tool in the physiopy suite.

phys2bids contains a lot of good code that could be useful in the development of these other tools. We could copy-paste it, but that would make bug fixes and code maintenance much harder. We could, alternatively, add it as a dependency in these new tools, and import those functions and classes that we need from it. However, this would tie up phys2bids developments with the other tools, possibly limiting the integration of cool, new features (Eyetracking anyone?), or making it a “heavier” dependency package that would become a burden to import/install. Another possibility would be to collect all the code that could be useful in other projects and move it to a repository on its own (e.g., physutils). For instance, phys2bids.utils contains a lot of general code that is not specific to phys2bids. Our BlueprintOutput class in phys2bids.physio_obj is a class that could be used as the main object for all the other developments.

I chatted a bit about this last idea with @rmarkello, and we thought it could work; however, we were wondering how to decide which code should go in physutils: should it be all the code that is used by at least two repositories, or should it be utilities that have a common scope (e.g. peak detection functions or visualisation or GLM implementations). It’s possible that, as we split phys2bids, maintaining code across two repositories might become a bit more confusing. However, it could decrease testing time for the phys2bids repository, offer a way to provide better maintenance for common utilities, and let each tool in the suite take its own path. Moreover, most of the code that could be moved hasn’t been modified since the main refactoring and last major release, as our development is taking place in other phys2bids-specific parts of the package.

Proposed solution

Outstanding questions

  1. What should we define as common code? Just the barebone code common to multiple repositories? Or a broader set of utilities that don’t deal with specific cases but could be potentially used everywhere - à la nibabel.
  2. Does anyone have a different proposal on how to approach this situation?
  3. Do you think there are some drawbacks that we’re not considering?

What does everyone think? (Pinging @vinferrer @rmarkello @RayStick @eurunuela as main code contributors, but everyone is welcome to provide feedback!)

Thanks to @rmarkello for revisions and suggestions

eurunuela commented 4 years ago

First of all, thanks for taking the time to bring up this discussion and creating the new repos!

I believe physutils is definitely the way to go. Once we're sure what parts of the code go into this repo, it should make development much easier.

Regarding what should go into physutils, I think it should be both utilities that are used in at least two repos and other utilities that could potentially be used everywhere. Not only will it make debugging easier, but it will also make developing new functionalities easier as we will know that physutils contains all the necessary stuff.

RayStick commented 4 years ago

Thanks for your thoughts.

I like the idea of separating out more ‘stand-alone’ code that phys2bids currently uses, and making it more generalizable in physutils. I don’t have too many strong opinions on how to go about that so going with the option that is easiest to manage and to debug makes sense.


Possible drawback to consider - how would this change the (new) user experience of using phys2bids (or other upcoming repos).

I also have scripts to create CO2, O2, Cardiac, Respiratory and RETROICOR regressors, and scripts that do cerebrovascular reactivity mapping - these are in MATLAB and BASH though (with AFNI and FSL commands), with no Python. Therefore, where I will likely be most useful is in contributing ideas/workflows and reviewing and testing the new utilities (based on your existing python code @smoia).