[MAINT] Store 1 timepoint for 4D generating a report with the maskers

[ymzayek]

• Closes Store one time point for reporting that uses 4D images #3933

Changes proposed in this pull request:

[github-actions]

👋 @ymzayek Thanks for creating a PR!

Until this PR is ready for review, you can include the [WIP] tag in its title, or leave it as a github draft.

Please make sure it is compliant with our contributing guidelines. In particular, be sure it checks the boxes listed below.

• PR has an interpretable title.
• PR links to Github issue with mention Closes #XXXX (see our documentation on PR structure)
• Code is PEP8-compliant (see our documentation on coding style)
• Changelog or what's new entry in doc/changes/latest.rst (see our documentation on PR structure)

For new features:

• There is at least one unit test per new function / class (see our documentation on testing)
• The new feature is demoed in at least one relevant example.

For bug fixes:

• There is at least one test that would fail under the original bug conditions.

We will review it as quick as possible, feel free to ping us with questions if needed.

[ymzayek]

This fails because the multi subject maskers don't support report generation. The same failure can be seen on main if calling generate_report on a multi masker object. So the question is do we want to support this for the multi subject maskers? Either way I think the error needs to be improved so I can do that in this PR and can open another issue to see if we would want to support this.

[bthirion]

Indeed, we should have report generation for multi subject maskers.

[ymzayek]

Ok I tried a few things to make this PR work and not break anything but I think a better strategy might be to better support report generation for the multi subject maskers before doing this. This is because the user could subscript the 5D list and generate_report will work as intended and trying to add a warning message about reports in fit will probably seem weird to users

[bthirion]

Ok I tried a few things to make this PR work and not break anything but I think a better strategy might be to better support report generation for the multi subject maskers before doing this. This is because the user could subscript the 5D list and generate_report will work as intended and trying to add a warning message about reports in fit will probably seem weird to users

Makes sense.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a24bf42) 91.64% compared to head (c3bad58) 91.69%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3935      +/-   ##
==========================================
+ Coverage   91.64%   91.69%   +0.05%     
==========================================
  Files         143      144       +1     
  Lines       16113    16150      +37     
  Branches     3353     3359       +6     
==========================================
+ Hits        14767    14809      +42     
+ Misses        798      796       -2     
+ Partials      548      545       -3     

Flag	Coverage Δ
macos-latest_3.10	91.60% <100.00%> (?)
macos-latest_3.11	91.60% <100.00%> (?)
macos-latest_3.12	91.60% <100.00%> (?)
macos-latest_3.9	91.57% <100.00%> (?)
ubuntu-latest_3.10	91.60% <100.00%> (?)
ubuntu-latest_3.11	91.60% <100.00%> (?)
ubuntu-latest_3.12	91.60% <100.00%> (?)
ubuntu-latest_3.9	91.57% <100.00%> (+0.05%)	⬆️
windows-latest_3.10	91.56% <100.00%> (?)
windows-latest_3.11	91.56% <100.00%> (+0.05%)	⬆️
windows-latest_3.12	91.56% <100.00%> (?)
windows-latest_3.9	91.53% <100.00%> (?)

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

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

[bthirion]

Technically, the PR looks good.

[ymzayek]

I definitely still need to add tests to this but I think it is finally in good shape for what it's intended to do

[ymzayek]

@Remi-Gau have you seen similar failures to the ones on windows here? I can't easily see how they are related but they are consistently failing so there is definitely something there

[Remi-Gau]

All of those are related to the use of the write_tmp_imgs context manager?

[Remi-Gau]

I think that we should probably start removing this context manager in the tests and replace it with a just creating the files in the tmp_path.

[ymzayek]

Seems to also happen with write_fake_fmri_data_and_design. Ok I'll see if rewriting the tests without the context manager resolves this

[Remi-Gau]

I am struggling to see what in the PR would start triggering those failures though.

[ymzayek]

Ok tests related to write_tmp_imgs are not failing anymore with the refactoring. write_fake_fmri_data_and_design seems to still be causing issues

[bthirion]

LGTM.

[Remi-Gau]

good with me