Add Physio object generation from BIDS

maestroque commented 4 months ago

Closes #3

Proposed Changes

Add a function to load BIDS structures and automatically generate Physio objects
Add an MRI data object to store trigger parameters, universal to all generated Physio objects

Change Type

[ ] bugfix (+0.0.1)
[X] minor (+0.1.0)
[ ] major (+1.0.0)
[ ] refactoring (no version update)
[ ] test (no version update)
[ ] infrastructure (no version update)
[ ] documentation (no version update)
[ ] other

Checklist before review

[ ] I added everything I wanted to add to this PR.
[ ] [Code or tests only] I wrote/updated the necessary docstrings.
[ ] [Code or tests only] I ran and passed tests locally.
[ ] [Documentation only] I built the docs locally.
[x] My contribution is harmonious with the rest of the code: I'm not introducing repetitions.
[x] My code respects the adopted style, especially linting conventions.
[x] The title of this PR is explanatory on its own, enough to be understood as part of a changelog.
[ ] I added or indicated the right labels.
[ ] I added information regarding the timeline of completion for this PR.
[ ] Please, comment on my PR while it's a draft and give me feedback on the development!

m-miedema commented 4 months ago

See my comment on the corresponding issue, I think we'll need to add more flexibility to column naming for this to be useful.

maestroque commented 4 months ago

@m-miedema it's fixed for the time being. I have implemented string literals searching, to give the physio_type property that is used in some functions.

m-miedema commented 4 months ago

This looks reasonable to me! Let me know when it's ready for review :)

me-pic commented 4 months ago

Hi @maestroque ! For the addition of unit tests for those new functions, I've talked with a colleague and she recommended to just generate the structure on the fly when we run pytest (and generate random number for the timeseries if we even need to), if we are just concerned about the data structure ! If we are more interested about the content of those files and need actual real data you can take a look at that BIDS app

maestroque commented 4 months ago

Thanks @me-pic ! Sounds reasonable to me

maestroque commented 3 months ago

@me-pic the unit test is ready. I'll address your other comments shortly and it should be good

maestroque commented 3 months ago

@me-pic I integrated your comments, so it should be good from that side!

@m-miedema I also addressed your comments on StartTime and n_slices. Regarding the use of the MRIConfig class and Physio, what you say is true, that was the intended use of the new class as we discussed in the past. It is pending the extraction of data from the trigger channel and the integration into the Physio object. I will open an issue about this, but I will tackle it if time allows after the workflow

maestroque commented 3 months ago

Other than that, it should be gtg

maestroque commented 3 months ago

@me-pic I think we are now ready, let me know if you have anything else in mind

m-miedema commented 3 months ago

Great work, thanks @maestroque !

smoia commented 3 months ago

:rocket: PR was released in 0.2.1 :rocket:

physiopy / physutils