Nilearn: Unclear bandpass filter application in `signal.clean`

[htwangtw]

I found the following lines of code a bit confusing:

https://github.com/nilearn/nilearn/blob/5f1cbf3645ae4f927857ff30db3e3121573020e3/nilearn/signal.py#L589-L599

Almost immediate after this, confound regression was performed. However, inside the statement triggering confound regression, we have the duplicated code block:
https://github.com/nilearn/nilearn/blob/5f1cbf3645ae4f927857ff30db3e3121573020e3/nilearn/signal.py#L606-L619

I think it is possible to apply butterworth filter twice if the user provide extra confound regressors.
@jerdra has done a lot of work related to filtering and signal handling, I would like you to double check on this issue.

There are other duplications that doesn't effect the output (the t_r argument check in L586 and L594)
Overall, I think singal.clean can be benefit from refactoring so we can better address future changes.
Please let me know what you think.

[htwangtw]

@jerad has kindly pointed out that I can't read :see_no_evil:
The the first block was done on the signal and the second one is on confounds.

Still, it will be helpful to refactor it so we can address #1011 in with a clearer idea how the data flows .

[NicolasGensollen]

@htwangtw as you said during your talk, this function is complicated and the order in which operations are made is extremely important even if it is not really documented.
Every time I have to touch it I'm afraid of breaking something major... :sweat:

[htwangtw]

I had a go already and all the tests related to signal.py still stands, just about to open a separate issue, or we can just revive and repurpose this one?

[NicolasGensollen]

@htwangtw you can reopen one if the purpose isn't the same. Will be easier for people to quickly get the context.