nipreps / niworkflows

Common workflows for MRI (anatomical, functional, diffusion, etc)
https://www.nipreps.org/niworkflows
Apache License 2.0
87 stars 52 forks source link

FIX: Use len(segments) to avoid nsegments becoming wrong #795

Closed effigies closed 1 year ago

effigies commented 1 year ago

Another in the series of handling carpetplots in degenerate cases that pretty much only come up on CI. Here, we have a variable nsegments = len(segments), but segments can be pared down if there are empty segments.

<100ns to call len() on a dict 4 times seems like a good trade over possibly getting de-synced again in the future.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -10.70 :warning:

Comparison is base (10e8c96) 72.96% compared to head (f36ffea) 62.27%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #795 +/- ## =========================================== - Coverage 72.96% 62.27% -10.70% =========================================== Files 76 49 -27 Lines 7547 5967 -1580 Branches 883 883 =========================================== - Hits 5507 3716 -1791 - Misses 2004 2071 +67 - Partials 36 180 +144 ``` | Flag | Coverage Δ | | |---|---|---| | reportlettests | `?` | | | unittests | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/nipreps/niworkflows/pull/795?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps) | Coverage Δ | | |---|---|---| | [niworkflows/viz/plots.py](https://codecov.io/gh/nipreps/niworkflows/pull/795?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps#diff-bml3b3JrZmxvd3Mvdml6L3Bsb3RzLnB5) | `68.14% <100.00%> (-8.12%)` | :arrow_down: | ... and [52 files with indirect coverage changes](https://codecov.io/gh/nipreps/niworkflows/pull/795/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipreps)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

effigies commented 1 year ago

I assume no objections and will merge in the morning, but feel free to make any opinions known before then.