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: Output TaskName and timing metadata for all resampled BOLD series #2320

Merged
merged 5 commits into from
Nov 6, 2020

Conversation

effigies
Copy link
Member

@effigies effigies commented Nov 5, 2020

Changes proposed in this pull request

  • Add a function to determine output timing parameters from input metadata.
  • Pass timing parameters to all BOLD series data sinks to ensure consistent metadata
  • Also adds TaskName to relevant data sinks

Closes #1549.
Fixes #2193.
Fixes #2294.

Depends on nipreps/niworkflows#578.

Documentation that should be reviewed

@pep8speaks
Copy link

pep8speaks commented Nov 5, 2020

Hello @effigies! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-11-05 21:31:13 UTC

@effigies
Copy link
Member Author

effigies commented Nov 5, 2020

Need to rerun to check expected outputs, but the JSON files can be inspected in artifacts: https://app.circleci.com/pipelines/github/nipreps/fmriprep/51/workflows/a89942cc-7cd9-4b18-acbe-b83c777304b5/jobs/324/artifacts

fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-1_space-fsLR_den-91k_bold.json:

{
  "RepetitionTime": 2.0,
  "TaskName": "mixed-gambles task"
}

fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-1_space-fsLR_den-91k_bold.dtseries.json:

{
  "grayordinates": "91k",
  "space": "HCP grayordinates",
  "surface": "fsLR",
  "surface_density": "32k",
  "volume": "MNI152NLin6Asym"
}

@effigies
Copy link
Member Author

effigies commented Nov 5, 2020

Kind of wild RTD build:

Collecting psutil>=5.4
  Downloading psutil-5.7.3.tar.gz (465 kB)
Collecting pyyaml
  Downloading PyYAML-5.3.1.tar.gz (269 kB)
Collecting sdcflows~=1.3.2
  Downloading sdcflows-1.3.3-py3-none-any.whl (5.6 MB)
Collecting niworkflows@ git+https://github.com/effigies/niworkflows.git@fix/legacy_dtseries_json
  Cloning https://github.com/effigies/niworkflows.git (to revision fix/legacy_dtseries_json) to /tmp/pip-install-7hlb83i3/niworkflows_7dc7f1f41a2b431c830c641d336b357d
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
    Preparing wheel metadata: started
    Preparing wheel metadata: finished with status 'done'
Collecting sdcflows~=1.3.2
  Downloading sdcflows-1.3.2-py3-none-any.whl (5.6 MB)
Collecting pyyaml
  Downloading PyYAML-5.3.tar.gz (268 kB)
Collecting pyyaml
  Downloading PyYAML-5.2.tar.gz (265 kB)
Collecting pyyaml
  Downloading PyYAML-5.1.2.tar.gz (265 kB)
Collecting pyyaml
  Downloading PyYAML-5.1.1.tar.gz (274 kB)
Collecting pyyaml
  Downloading PyYAML-5.1.tar.gz (274 kB)
Collecting pyyaml
  Downloading PyYAML-3.13.tar.gz (270 kB)
Collecting pyyaml
  Downloading PyYAML-3.12.zip (375 kB)
Collecting pyyaml
  Downloading PyYAML-3.11.zip (371 kB)
Collecting pyyaml
  Downloading PyYAML-3.10.zip (364 kB)

@effigies effigies force-pushed the fix/legacy_dtseries_json branch from 4ffd3a3 to e97b547 Compare November 5, 2020 21:31
@effigies
Copy link
Member Author

effigies commented Nov 5, 2020

Niworkflows 1.3.2 is out. This should be ready for review. (Apologies if CircleCI comes back in 2 hours to prove me wrong...)

@effigies
Copy link
Member Author

effigies commented Nov 6, 2020

All green. @bpinsard @mgxd @oesteban Are we happy with this for 20.2.1?

Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much needed. Thanks much, I'm totally in favor of including this within the LTS.

@effigies effigies merged commit 11dffb1 into nipreps:maint/20.2.x Nov 6, 2020
@effigies effigies deleted the fix/legacy_dtseries_json branch November 6, 2020 13:12
@bpinsard
Copy link
Collaborator

bpinsard commented Nov 6, 2020

Great! Looks all good!

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.

4 participants