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

Backport 2.2.0 PRs into 2.1.x branch #316

Closed
wants to merge 12 commits into from

Conversation

effigies
Copy link
Member

@effigies effigies commented Dec 7, 2022

Backports #257, #284, #287, #288, #292, #293, #299, #310.

All of these should be uncontroversial to have in a bug-fix release. The main goal is to have a recent CircleCI run before a lot of changes to ensure that we're not regressing in artifacts.

mgxd and others added 10 commits December 7, 2022 13:41
The ``sloppy`` option was misused for debugging (also because the name
was misused).

This PR makes a better use of debug/sloppy boolean arguments.

Backport niprepsgh-287
Backport niprepsgh-288
This is to bring SDCFlow's file filtering more in-line with approaches taken in other nipreps.
To avoid potential collisions during `get()` parameter assigment, base / additional values are first merged
into a dictionary, and then passed in as keyword arguments

Backport niprepsgh-292
@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

Now also backporting #270, #277, #281, #282, #313

@codecov-commenter
Copy link

Codecov Report

Base: 83.03% // Head: 88.47% // Increases project coverage by +5.44% 🎉

Coverage data is based on head (e3411c8) compared to base (c85a14d).
Patch coverage: 92.92% of modified lines in pull request are covered.

Additional details and impacted files
@@               Coverage Diff               @@
##           maint/2.1.x     #316      +/-   ##
===============================================
+ Coverage        83.03%   88.47%   +5.44%     
===============================================
  Files               25       25              
  Lines             1851     1927      +76     
  Branches           278      289      +11     
===============================================
+ Hits              1537     1705     +168     
+ Misses             282      188      -94     
- Partials            32       34       +2     
Flag Coverage Δ
travis 85.44% <92.92%> (+2.40%) ⬆️
unittests 88.37% <92.92%> (?)

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

Impacted Files Coverage Δ
sdcflows/workflows/apply/registration.py 92.00% <ø> (ø)
sdcflows/cli/find_estimators.py 89.36% <89.36%> (ø)
sdcflows/utils/wrangler.py 95.41% <93.61%> (-2.12%) ⬇️
sdcflows/fieldmaps.py 99.41% <100.00%> (+<0.01%) ⬆️
sdcflows/transform.py 91.30% <100.00%> (ø)
sdcflows/utils/misc.py 100.00% <100.00%> (ø)
sdcflows/__init__.py
sdcflows/workflows/outputs.py 93.54% <0.00%> (+1.61%) ⬆️
sdcflows/interfaces/reportlets.py 89.83% <0.00%> (+1.69%) ⬆️
sdcflows/interfaces/bspline.py 87.44% <0.00%> (+4.18%) ⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

Did we resolve a L/R flip in 2.2.0? Here's tmp/tests/test-fieldmap-masked.svg in this 2.1.x PR (left) and master (right).

image

@effigies
Copy link
Member Author

effigies commented Dec 7, 2022

Guessing this happened in #278, but will need input from @oesteban to verify that this is expected. The artifacts are gone, so I can't compare that PR immediately.

@oesteban
Copy link
Member

I believe we have tests to check we are interpreting the output of TOPUP correctly. This is probably a flip in plotting rather than correction. Unfortunately, I would need time to check this more thoroughly - but your guess about #278 is very likely to be the source of the flip.

@effigies
Copy link
Member Author

effigies commented Jan 4, 2023

No 2.1.2 release is planned.

@effigies effigies closed this Jan 4, 2023
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