Skip to content

REF: Avoid divide by zero warnings when denominator is zero#440

Merged
jhlegarreta merged 4 commits into
nipreps:mainfrom
jhlegarreta:ref/avoid-divide-by-zero
Apr 30, 2026
Merged

REF: Avoid divide by zero warnings when denominator is zero#440
jhlegarreta merged 4 commits into
nipreps:mainfrom
jhlegarreta:ref/avoid-divide-by-zero

Conversation

@jhlegarreta
Copy link
Copy Markdown
Contributor

Avoid divide by zero warnings when denominator is zero: check that values are nonzero before dividing; else, set all values to zero.

Add the corresponding tests.

Fixes:

test/test_integration.py: 15 warnings
    /home/runner/work/nifreeze/nifreeze/src/nifreeze/data/filtering.py:107:
    RuntimeWarning: invalid value encountered in divide
          data /= data.max()

raised for example at:
https://github.com/nipreps/nifreeze/actions/runs/24288044855/job/70920601980?pr=313

Avoid divide by zero warnings when denominator is zero: check that
values are nonzero before dividing; else, set all values to zero.

Add the corresponding tests.

Fixes:
```
test/test_integration.py: 15 warnings
    /home/runner/work/nifreeze/nifreeze/src/nifreeze/data/filtering.py:107:
    RuntimeWarning: invalid value encountered in divide
          data /= data.max()
```

raised for example at:
https://github.com/nipreps/nifreeze/actions/runs/24288044855/job/70920601980?pr=313
@jhlegarreta jhlegarreta force-pushed the ref/avoid-divide-by-zero branch from ab8b0a6 to 0344cee Compare April 11, 2026 23:51
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.46%. Comparing base (0b51bd4) to head (04c7006).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #440      +/-   ##
==========================================
+ Coverage   84.30%   84.46%   +0.16%     
==========================================
  Files          37       37              
  Lines        2134     2156      +22     
  Branches      239      243       +4     
==========================================
+ Hits         1799     1821      +22     
  Misses        296      296              
  Partials       39       39              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@arokem arokem left a comment

Choose a reason for hiding this comment

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

Good idea, but a couple of comments on implementation

Comment thread src/nifreeze/data/filtering.py Outdated
Comment thread src/nifreeze/data/filtering.py Outdated
Rename variable to avoid name ambiguity with the random number generator variable name and improve clarity.

Co-authored-by: Ariel Rokem <arokem@gmail.com>
@jhlegarreta jhlegarreta force-pushed the ref/avoid-divide-by-zero branch from da62de7 to f0469ce Compare April 14, 2026 01:58
Raise typed `ClippingValueError` for `advanced_clip` degeneracies:
`advanced_clip()` now fails fast with explicit errors for:
- empty percentile selection after applying nonnegative/finite constraints
- non-finite percentile thresholds
- invalid percentile thresholds (`a_max <= a_min`)
- degenerate dynamic range after clipping/shift (`den == 0` or non-finite)

Add the corresponding unit tests.
@jhlegarreta jhlegarreta force-pushed the ref/avoid-divide-by-zero branch from f0469ce to 7a4ec7f Compare April 14, 2026 02:03
@jhlegarreta jhlegarreta requested a review from oesteban April 29, 2026 09:36
@jhlegarreta jhlegarreta merged commit dc42626 into nipreps:main Apr 30, 2026
10 of 11 checks passed
@jhlegarreta jhlegarreta deleted the ref/avoid-divide-by-zero branch April 30, 2026 09:08
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.

3 participants