Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanding the physiopy suite #186

Open
smoia opened this issue Mar 25, 2020 · 2 comments
Open

Expanding the physiopy suite #186

smoia opened this issue Mar 25, 2020 · 2 comments
Assignees
Labels
Community This issue is about the community, not the code Discussion Discussion of a concept or implementation. Need to stay always open. Refactoring Improve nonfunctional attributes

Comments

@smoia
Copy link
Member

smoia commented Mar 25, 2020

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

  • Move utils.py, the BlueprintOutput class in physio_obj.py, and everything related to them (tests, dedicated functions, related contributions, etc.), and move them to a new repository
  • Import such repository where needed into phys2bids and have it as a required dependency

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

@smoia smoia added Discussion Discussion of a concept or implementation. Need to stay always open. Refactoring Improve nonfunctional attributes Community This issue is about the community, not the code labels Mar 25, 2020
@smoia smoia self-assigned this Mar 25, 2020
@eurunuela
Copy link
Collaborator

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.

@smoia smoia mentioned this issue Mar 26, 2020
2 tasks
@RayStick
Copy link
Member

RayStick commented Apr 1, 2020

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community This issue is about the community, not the code Discussion Discussion of a concept or implementation. Need to stay always open. Refactoring Improve nonfunctional attributes
Projects
None yet
Development

No branches or pull requests

3 participants