[doc] Warning for non-matching n_parcels

[emdupre]

Closes #2238.

• Adds a warning when learned n_parcels does not match requested n_parcels

[codecov]

Codecov Report

Merging #2240 into master will decrease coverage by 0.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2240      +/-   ##
==========================================
- Coverage    92.7%   92.57%   -0.13%     
==========================================
  Files         150      150              
  Lines       19019    19038      +19     
  Branches     2311     2313       +2     
==========================================
- Hits        17632    17625       -7     
- Misses        883      905      +22     
- Partials      504      508       +4

Impacted Files	Coverage Δ
nilearn/regions/tests/test_parcellations.py	100% <100%> (ø)	⬆️
nilearn/regions/parcellations.py	96.36% <100%> (+0.17%)	⬆️
nilearn/reporting/rm_file.py	26.66% <0%> (-53.34%)	⬇️
nilearn/_utils/testing.py	79.57% <0%> (-4.93%)	⬇️
nilearn/plotting/__init__.py	95.65% <0%> (-4.35%)	⬇️
nilearn/plotting/tests/test_js_plotting_utils.py	97.6% <0%> (-1.6%)	⬇️
nilearn/datasets/tests/test_neurovault.py	95.84% <0%> (-0.98%)	⬇️
nilearn/image/image.py	95.37% <0%> (-0.72%)	⬇️

---

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 da84a26...ab8cfad. Read the comment docs.

[kchawla-pi]

Nice! A test & what's new entry & this should be ready.

[emdupre]

@GaelVaroquaux I was looking at adding another test, to confirm that the warning isn't raised when the n_parcels number matches. Here's what I had:

@pytest.mark.filterwarnings('ignore::DeprecationWarning')
def test_parcellations_no_warnings():
    data = np.zeros((10, 11, 12, 5))
    img = nibabel.Nifti1Image(data, affine=np.eye(4))
    parcellator_n7 = Parcellations(method='kmeans', n_parcels=1)
    with pytest.warns(None) as record:
        parcellator_n7.fit(img)
    assert record.list is None

The problem is that DeprecationWarning's still aren't being filtered. It looks to be related to this point.

Do you have any suggestions of how to filter appropriately and confirm there's no UserWarning, but other (deprecation) warnings are fine ?

[GaelVaroquaux]

assert record.list is None

I would filter out the list manually here.

[kchawla-pi]

Hi @emdupre
What I suggest is to simply use the match parameter in the pytest.warns(match=) to specifically match the string from your warning to ensure other UserWarnings are not caught by mistake.

Also instead of doing what you did above, you can write another test like the one you have added in the code, but engineered such that the number of parcels are the same, and mark it as expected to fail, using @pytest.mark.xfail

[GaelVaroquaux]

Once CI is green, I am +1 for merge.

[emdupre]

Thanks, @kchawla-pi and @GaelVaroquaux !

Hopefully this will pass as-is, but the different methods are good to know moving forward 😸

[kchawla-pi]

Totally. I particularly love pytest.warn(match=' string'). It is so much more compact & cleaner than the built in Python one.

[kchawla-pi]

I would filter out the list manually here.

In my experience the manual parsing approach is rather fragile, and when implemented in a robust way, looks ugly. Do consider my suggestion, I think that approach is clean and robust.

[GaelVaroquaux]

LGTM. Thank you for your perseverence

[kchawla-pi]

@emdupre I played around with Pytest to see if we can find ways to improve the testing. It seems to me my suggestion has gaps that pytest does not handle (counter-intuitive, hope they fix it), and your way is the better one.