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] accepts --output-layout argument #3

Merged
merged 8 commits into from
Jul 17, 2023

Conversation

zhao-cy
Copy link
Contributor

@zhao-cy zhao-cy commented Apr 13, 2023

BIDS Apps like fMRIPrep and QSIPrep would create a new folder within the designated output folder, e.g., new folders like fmriprep and freesurfer for fMRIPrep, and qsiprep for QSIPrep. So I added creating fmriprepfake folder here.

I've updated the version to 0.1.1 and pushed to my personal docker hub repo. For other changes please see the updated README.md.

Notes added on 7/13/2023: After talking w/ Dorota, I realized that this is not necessary the case for fMRIPrep, esp its recent updated output layout.

@zhao-cy
Copy link
Contributor Author

zhao-cy commented Apr 27, 2023

TODO for myself: add --output-layout legacy option (which will follow fmriprep's legacy output layout) before merging this PR.

@zhao-cy
Copy link
Contributor Author

zhao-cy commented Jul 11, 2023

In this PR, I made several enhancements regarding output layout and Docker image:

  • fmriprep-fake accepts argument --output-layout now. - this will close issue [ENH] add support for --output-layout argument in fMRIPrep #5
    • Currently --output-layout bids (or not to specify) would work similar to latest fMRIPrep's output layout;
    • whereas --output-layout legacy is different from how fMRIPrep would generate when legacy, i.e., two folders: fmriprep and freesurfer. Code for bids is largely reused for this case.
  • Generates log folder (within sub-* folder) now. This log folder is different from logs folder.
  • Docker image uses multi-arch to build, so the Docker image would also work on Mac M1 laptops.
  • README.md in the github repo has been updated accordingly.

@zhao-cy zhao-cy changed the title [ENH] added output folder fmriprepfake [ENH] accepts --output-layout argument Jul 13, 2023
@djarecka
Copy link
Owner

djarecka commented Jul 14, 2023

@zhao-cy - thank you for the changes!

Could you please add to the README how people should use the 2 scripts you added: prep_docker and test

@zhao-cy
Copy link
Contributor Author

zhao-cy commented Jul 14, 2023

Yes definitely! I'll also update the README.md too! I'll let you know once I think it's ready to merge.

@zhao-cy
Copy link
Contributor Author

zhao-cy commented Jul 14, 2023

Currently this updated version 0.1.2 is available on my personal docker hub: chenyingzhao/fmriprep_fake, but I think it would be better to have the latest version on your account. If you do so, as you'll re-build the docker image, I may recommend incrementing the version to 0.1.3, so that we can make sure for each version number, there is only one possible version of Docker image. Let me know what you think?

@zhao-cy
Copy link
Contributor Author

zhao-cy commented Jul 14, 2023

@djarecka I think this is ready to merge! Please note my previous comments above. Thanks!

@djarecka djarecka merged commit 95479bf into djarecka:master Jul 17, 2023
@djarecka
Copy link
Owner

thank you @zhao-cy . I will test it on openmind and create a new image on my dockerhub this week

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.

None yet

2 participants