-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add multi-run workflow synchronized with BIDS dataset #374
Conversation
Codecov Report
@@ Coverage Diff @@
## master #374 +/- ##
===========================================
- Coverage 94.61% 76.83% -17.79%
===========================================
Files 8 9 +1
Lines 836 1079 +243
===========================================
+ Hits 791 829 +38
- Misses 45 250 +205
|
Also try to use scans.tsv file when available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is a big one - I didn't notice it in #219 !
Thank you for making the review easier @tsalo, I'd love to see this in action sooner than what we'd have been able to otherwise!
This is not a complete review, I will need much more time to go through the whole code and check everything. However, I did add a couple of comments here and there that I'd ask in a review later on, so if you want you can check them out now and start reply to/address them.
I'll also leave the tests to test-savvy peeps around.
@@ -290,3 +291,99 @@ def readme_file(outdir): | |||
LGR.warning('phys2bids could not find README,' | |||
'generating it EMPTY, please fill in the necessary info') | |||
utils.write_file(file_path, '', text) | |||
|
|||
|
|||
def update_bids_name(basename, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eh, with nipy/heudiconv#544 I feel that we are doomed to start some minibids
or bidsnonofficial
or bidspy
(so it is not pybids
), whatever, to start with a basic schema-driven bids filenames support and may be later expanded to something else (validator etc). otherwise -- we keep breeding duplicated code across the ecosystem, not good! WDYT @tsalo ? I can move that heudiconv PR into its own python package, will you join the fun? ;)
@smoia I've realized over time that, not only is FIU's current physio setup suboptimal, but it's also pretty rare. I don't think that many folks would actually find this workflow useful, and I only ended up using it for one project (FIU's now in the process of updating their physio setup). Do you mind if I close this PR? |
Closes #217 and supersedes #219.
This is a reduced version of #219, with all extraneous changes removed. Here are the general steps for the proposed workflow:
BlueprintInput
object.BlueprintOutput
s.BlueprintOutput
objects to BIDS files.Proposed Changes
phys2bids.synchronization
, with a function namedworkflow()
meant to perform unsupervised separation of multi-run physio files using onset-time and duration information from a converted BIDS dataset.phys2bids.bids.update_bids_name()
, which adds entities to a BIDS-format filename.phys2bids.slice4phys.slice_phys()
, a function that slices a physio object into individual runs. This is based off ofphys2bids.slice4phys.slice4phys()
, but is not linked to the existing multi-run workflow.scipy
,pybids
, andpandas
.Change Type
bugfix
(+0.0.1)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