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

RF: Replace most of anat_ribbon_wf with a Python function #363

Merged
merged 10 commits into from
Aug 29, 2023

Conversation

effigies
Copy link
Member

Got sidetracked and rewrote all of anat_ribbon_wf except the creating signed distance volumes in Python. Wrote unit and integration test to verify that we do get the same results before replacing.

@effigies effigies force-pushed the enh/simplify-ribbon branch 2 times, most recently from abb4bdc to 663b136 Compare August 28, 2023 17:44
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Patch coverage: 78.57% and project coverage change: -24.60% ⚠️

Comparison is base (244f1b2) 54.00% compared to head (fb05822) 29.41%.

Additional details and impacted files
@@             Coverage Diff             @@
##             next     #363       +/-   ##
===========================================
- Coverage   54.00%   29.41%   -24.60%     
===========================================
  Files          23       20        -3     
  Lines        1685     1676        -9     
  Branches      229      231        +2     
===========================================
- Hits          910      493      -417     
- Misses        719     1170      +451     
+ Partials       56       13       -43     
Flag Coverage Δ
ds005 ?
ds054 ?
pytest ?

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

Files Changed Coverage Δ
smriprep/workflows/anatomical.py 12.12% <ø> (-32.96%) ⬇️
smriprep/workflows/surfaces.py 13.04% <33.33%> (+0.95%) ⬆️
smriprep/interfaces/surf.py 53.38% <84.00%> (+9.17%) ⬆️

... and 14 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@effigies
Copy link
Member Author

Noting here that the test passed with the old workflow.

smriprep/interfaces/surf.py Outdated Show resolved Hide resolved
@effigies
Copy link
Member Author

@mgxd I would appreciate a review of this. The regression test that verifies consistent behavior is the main thing.

smriprep/interfaces/surf.py Outdated Show resolved Hide resolved
smriprep/workflows/surfaces.py Show resolved Hide resolved
effigies and others added 2 commits August 29, 2023 13:32
Co-authored-by: Mathias Goncalves <goncalves.mathias@gmail.com>
@mgxd
Copy link
Collaborator

mgxd commented Aug 29, 2023

dunno why cache failed, but this looks good to me

@effigies effigies merged commit 9a135f4 into nipreps:next Aug 29, 2023
8 checks passed
@effigies effigies deleted the enh/simplify-ribbon branch August 29, 2023 19:52
@effigies effigies added the next label Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants