ENH: add rodent-specific adaption for B-spline #630

eilidhmacnicol · 2021-04-19T15:26:28Z

Addressing #611, this PR 1) adds a function to determine bspline grid using a ratio of the number of slices in each dimension, and 2) adds logic to various workflows so that this might be used with rodents by updating the N4 parameters. Not to say that the function can't be used with non-rodent images, but it hasn't been tested as such.

There are other workflows that call N4BiasFieldCorrection, but they are unlikely to be used by NiRodents/fprodents because there are already rodent alternatives or because they would require more changes and would probably benefit from a rodent-specific workflow.

pep8speaks · 2021-04-19T15:26:40Z

Hello @eilidhmacnicol, Thank you for updating!

In the file niworkflows/anat/ants.py:

Line 766:30: E226 missing whitespace around arithmetic operator

In the file niworkflows/utils/images.py:

Line 335:1: W293 blank line contains whitespace

In the file niworkflows/workflows/epi/refmap.py:

Line 169:30: E226 missing whitespace around arithmetic operator

To test for issues locally, pip install flake8 and then run flake8 niworkflows.

Comment last updated at 2021-11-17 17:19:15 UTC

codecov · 2021-04-19T16:48:02Z

Codecov Report

Merging #630 (638f459) into master (616d2bc) will decrease coverage by 1.20%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master     #630      +/-   ##
==========================================
- Coverage   48.86%   47.65%   -1.21%     
==========================================
  Files          45       45              
  Lines        5501     5523      +22     
  Branches      790      790              
==========================================
- Hits         2688     2632      -56     
- Misses       2712     2796      +84     
+ Partials      101       95       -6

Flag	Coverage Δ
masks	`?`
reportlettests	`∅ <ø> (∅)`
unittests	`47.65% <7.69%> (-0.19%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
niworkflows/anat/ants.py	`8.87% <0.00%> (-0.46%)`	⬇️
niworkflows/utils/images.py	`56.86% <11.11%> (-2.86%)`	⬇️
niworkflows/workflows/epi/refmap.py	`60.00% <20.00%> (-4.45%)`	⬇️
niworkflows/func/util.py	`17.97% <0.00%> (-31.47%)`	⬇️
niworkflows/interfaces/header.py	`51.70% <0.00%> (-11.12%)`	⬇️
niworkflows/interfaces/fixes.py	`50.00% <0.00%> (-5.56%)`	⬇️
niworkflows/interfaces/reportlets/base.py	`42.85% <0.00%> (-1.59%)`	⬇️
niworkflows/viz/utils.py	`8.84% <0.00%> (+0.02%)`	⬆️
niworkflows/interfaces/norm.py	`25.86% <0.00%> (+0.14%)`	⬆️
niworkflows/interfaces/images.py	`66.88% <0.00%> (+0.21%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 616d2bc...638f459. Read the comment docs.

niworkflows/utils/images.py

oesteban

A few suggestions

niworkflows/anat/ants.py

niworkflows/utils/images.py

eilidhmacnicol · 2021-07-27T17:01:25Z

Okay, tested the N4 in the reference workflow on two sets of multi-run EPIs from a subject, and it works well.
Although I was going to develop some tests for the bspline_grid function, I would quite like to push on with integrating the reference workflow into fmriprep-rodents.

@oesteban what do you think?

eilidhmacnicol · 2021-07-27T17:02:39Z

test_pytest seems to be failing because of the pandas versioning rather than anything to do with the code, FYI

oesteban · 2021-07-29T08:48:54Z

I'll have a look ASAP

oesteban

Looking good - left a few minor comments and nit picks

niworkflows/anat/ants.py

niworkflows/utils/images.py

niworkflows/workflows/epi/refmap.py

niworkflows/anat/ants.py

codecov-commenter · 2021-08-05T13:44:46Z

Codecov Report

Merging #630 (57621e4) into feat/infants-rodents (17c3b8a) will decrease coverage by 1.15%.
The diff coverage is 5.55%.

❗ Current head 57621e4 differs from pull request most recent head cc7ef7f. Consider uploading reports for the commit cc7ef7f to get more accurate results

@@                   Coverage Diff                    @@
##           feat/infants-rodents     #630      +/-   ##
========================================================
- Coverage                 49.02%   47.86%   -1.16%     
========================================================
  Files                        45       45              
  Lines                      5520     5534      +14     
  Branches                    795      794       -1     
========================================================
- Hits                       2706     2649      -57     
- Misses                     2713     2790      +77     
+ Partials                    101       95       -6

Flag	Coverage Δ
masks	`?`
reportlettests	`∅ <ø> (∅)`
unittests	`47.86% <5.55%> (-0.14%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
niworkflows/anat/ants.py	`9.16% <0.00%> (-0.16%)`	⬇️
niworkflows/workflows/epi/refmap.py	`58.00% <0.00%> (-6.45%)`	⬇️
niworkflows/utils/images.py	`58.49% <11.11%> (-2.85%)`	⬇️
niworkflows/func/util.py	`17.97% <0.00%> (-31.47%)`	⬇️
niworkflows/interfaces/header.py	`51.70% <0.00%> (-11.12%)`	⬇️
niworkflows/interfaces/fixes.py	`50.00% <0.00%> (-5.56%)`	⬇️
niworkflows/interfaces/reportlets/base.py	`42.85% <0.00%> (-1.59%)`	⬇️
niworkflows/viz/utils.py	`8.84% <0.00%> (+0.02%)`	⬆️
niworkflows/interfaces/norm.py	`25.86% <0.00%> (+0.14%)`	⬆️
niworkflows/interfaces/images.py	`66.88% <0.00%> (+0.21%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 17c3b8a...cc7ef7f. Read the comment docs.

oesteban · 2021-08-05T13:45:37Z

test_pytest seems to be failing because of the pandas versioning rather than anything to do with the code, FYI

I just rebased your branch on upstream/master where the problem has been recently addressed.

oesteban · 2021-08-05T13:48:26Z

I also have changed the target branch to feat/infants-rodents so that we don't mess up the release of fMRIPrep 21.0.x

Co-authored-by: Oscar Esteban <code@oscaresteban.es>

eilidhmacnicol · 2021-11-15T10:13:23Z

@oesteban seems like the issue that was causing the tests to fail was related to the nipype version. Pinning to master has bypassed this, so is this ready to merge or should we pin to a newer version of nipype first?

eilidhmacnicol · 2021-11-17T17:14:37Z

More fun - it seems that whatever is causing the tests to fail has been resolved somewhere between the release of nipype 1.7.0 (https://github.com/nipy/nipype/tree/b38572030f3d968c945675efaccd053d123f158b; released 28 days ago at time of writing) and the version accessed today (https://github.com/nipy/nipype/tree/f756b71117493caea093bac79d576207d8f96482).

My guess is that it is fixed by nipy/nipype#3395, so I guess we will have to pin to 1.7.1 when it is released.

eilidhmacnicol · 2021-11-17T17:16:40Z

In the meantime, I think I will revert to pinning to master and push through so we are not waiting solely on a nipype release

eilidhmacnicol requested a review from oesteban April 19, 2021 15:26

eilidhmacnicol linked an issue Apr 20, 2021 that may be closed by this pull request

Revise all N4 nodes and add adaptations for rodents #611

Open

eilidhmacnicol marked this pull request as draft April 20, 2021 08:23

eilidhmacnicol commented Apr 20, 2021

View reviewed changes

niworkflows/utils/images.py Show resolved Hide resolved

oesteban reviewed Jul 8, 2021

View reviewed changes

eilidhmacnicol marked this pull request as ready for review July 27, 2021 17:01

oesteban approved these changes Aug 4, 2021

View reviewed changes

eilidhmacnicol added 7 commits August 5, 2021 15:43

ENH: add function to determine grid size

f88a6c7

ENH: update relevant workflows

b0bdf83

FIX: appeasing linter

ce56223

FIX: options generalised beyond rodents

c5632f8

FIX: bspline grid func improvements

53f1e24

FIX: integrate suggestions from review

6d20a25

FIX: bspline grid input

018b3c3

oesteban force-pushed the enh/bspline_grid_logic branch from e0eb48d to 018b3c3 Compare August 5, 2021 13:44

oesteban changed the base branch from master to feat/infants-rodents August 5, 2021 13:47

wip: apply suggestions from code review

6b08035

Co-authored-by: Oscar Esteban <code@oscaresteban.es>

eilidhmacnicol mentioned this pull request Aug 17, 2021

Reference workflow integration nipreps/fmriprep-rodents#35

Closed

eilidhmacnicol added 2 commits October 29, 2021 16:09

Apply code review suggestions

3bfacb4

PIN: nipype master branch

57621e4

eilidhmacnicol force-pushed the enh/bspline_grid_logic branch from cc7ef7f to 57621e4 Compare November 17, 2021 17:19

eilidhmacnicol merged commit 9137e31 into nipreps:feat/infants-rodents Nov 17, 2021

eilidhmacnicol deleted the enh/bspline_grid_logic branch November 17, 2021 17:22

eilidhmacnicol mentioned this pull request Nov 17, 2021

ENH: reference workflow N4 improvements nipreps/fmriprep-rodents#40

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: add rodent-specific adaption for B-spline #630

ENH: add rodent-specific adaption for B-spline #630

eilidhmacnicol commented Apr 19, 2021

pep8speaks commented Apr 19, 2021 •

edited

Loading

codecov bot commented Apr 19, 2021 •

edited

Loading

oesteban left a comment

eilidhmacnicol commented Jul 27, 2021

eilidhmacnicol commented Jul 27, 2021

oesteban commented Jul 29, 2021

oesteban left a comment

codecov-commenter commented Aug 5, 2021 •

edited

Loading

oesteban commented Aug 5, 2021

oesteban commented Aug 5, 2021

eilidhmacnicol commented Nov 15, 2021

eilidhmacnicol commented Nov 17, 2021

eilidhmacnicol commented Nov 17, 2021

ENH: add rodent-specific adaption for B-spline #630

ENH: add rodent-specific adaption for B-spline #630

Conversation

eilidhmacnicol commented Apr 19, 2021

pep8speaks commented Apr 19, 2021 • edited Loading

Comment last updated at 2021-11-17 17:19:15 UTC

codecov bot commented Apr 19, 2021 • edited Loading

Codecov Report

oesteban left a comment

Choose a reason for hiding this comment

eilidhmacnicol commented Jul 27, 2021

eilidhmacnicol commented Jul 27, 2021

oesteban commented Jul 29, 2021

oesteban left a comment

Choose a reason for hiding this comment

codecov-commenter commented Aug 5, 2021 • edited Loading

Codecov Report

oesteban commented Aug 5, 2021

oesteban commented Aug 5, 2021

eilidhmacnicol commented Nov 15, 2021

eilidhmacnicol commented Nov 17, 2021

eilidhmacnicol commented Nov 17, 2021

pep8speaks commented Apr 19, 2021 •

edited

Loading

codecov bot commented Apr 19, 2021 •

edited

Loading

codecov-commenter commented Aug 5, 2021 •

edited

Loading