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

ENH: reference workflow 2.0 #38

Merged
merged 7 commits into from
Oct 29, 2021

Conversation

eilidhmacnicol
Copy link
Collaborator

@eilidhmacnicol eilidhmacnicol commented Oct 22, 2021

Changes proposed in this pull request

Unlike in #35, this reference workflow creates one reference image per BOLD run. In future, it would be good to implement something as in nipreps/nibabies#15 where reference images are calculated for similar BOLD runs (same PE, readout time) but this can be added at a later stage.

Documentation that should be reviewed

@eilidhmacnicol eilidhmacnicol force-pushed the enh/ref_workflow2 branch 2 times, most recently from aa1dffc to 9046e01 Compare October 22, 2021 16:18
@eilidhmacnicol
Copy link
Collaborator Author

The checks that are failing are only doing so on the install in confined environment - [ setup.py - install ] workflow. This seems to be because of a dependency clash between the updated nipreps/niworkflows#656. I think the order of installation proceeds with tedana dependencies being installed first, and the newest version of matplotlib is being installed as a consequence of their non-specific dependency, see here, which later conflicts with niworkflows pinned dependency.

@oesteban - I don't know how to proceed with this, but the circleCI checks are looking good with the workflow completing without errors. WDYT?

fprodents/workflows/base.py Show resolved Hide resolved
fprodents/workflows/base.py Outdated Show resolved Hide resolved
@eilidhmacnicol
Copy link
Collaborator Author

@oesteban - I think these changes are good to go; can we address the pip build before merging?

@oesteban
Copy link
Member

Okay, you should be able to rebase or merge upstream/master (don't forget to run git fetch upstream first)

@eilidhmacnicol
Copy link
Collaborator Author

eilidhmacnicol commented Oct 29, 2021

The removal of nitime from dependencies (after rebasing to master) caused the last crash.
nitime is used in the confounds workflow by nipype, and since this is common to fmriprep but nitime is not included in its dependencies, I suspect that nitime is a dependency for a package required for fmriprep but not fmriprep-rodents. I tried digging around but it wasn't immediately obvious so adding it back in for the time being.

Fingers crossed that this time we get a clean sheet

@eilidhmacnicol
Copy link
Collaborator Author

Ding Ding Ding! We have a winner.

I've noticed that our ME pipeline might need some work before release (e.g. the masking is still using niworkflows rather than nirodents) but merging this and will test on SE and ME data next week. 🎉

@eilidhmacnicol eilidhmacnicol merged commit 34b97d6 into nipreps:master Oct 29, 2021
@eilidhmacnicol eilidhmacnicol deleted the enh/ref_workflow2 branch October 29, 2021 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not run artsBrainExtraction twice - fast BOLD reference masking
2 participants