nilearn icon indicating copy to clipboard operation
nilearn copied to clipboard

[WIP] Method to easily visualize predicted ts and residuals

Open ferponcem opened this issue 6 months ago • 4 comments

  • Closes #5723

Changes proposed in this pull request:

  • Adding a method to retrieve predicted and residuals from the glm fit.
  • Adding a figure with two subplots: observed and predicted time series, and residuals.

Trying to emulate this example.

Work in progress !

TODO

  • [ ] add entry in the changelog (make sure to add yourself to the citation.cff) see: https://nilearn.github.io/dev/development.html#changelog
  • [ ] update the example https://nilearn.github.io/stable/auto_examples/04_glm_first_level/plot_predictions_residuals.html#plot-predicted-and-actual-time-series-for-6-most-significant-clusters so show of the function is used
  • [ ] nilearn can work with no plotting dependencies so the function has to throw an error when matplotlib is missing (there is function is_matplotlib_installed in nilearn._utils.helpers that should help with that) : there should be a test for this. Test to run only when maplotlib is missing can be marked by
@pytest.mark.skipif(
    is_matplotlib_installed(),
    reason="This test is run only if matplotlib is not installed.",
)
def test_something()
  • [ ] related to the point above, maybe we could have a function (maybe a one private for now) that returns dataframe or an array with predicted, observed and residuals that can work even if no plotting dependencies is installed and have the plotting function only get the data from this and just do the plotting (FYI: this can also make testing much easier)
  • [ ] check how the plots look when you pass more than one location (say 10) to plot (maybe worth checking how it behaves when also giving it a abel or maps masker): do we want all on one plot or one plot per ROI?
  • [ ] in terms of testing we are generating a figure, so it may be worth adding a baseline figure for this function to check that the way it looks stays constant: see https://nilearn.github.io/dev/maintenance.html#generating-new-baseline-figures-for-plotting-tests (I can take care of that or help you set it up if you want)

ferponcem avatar Oct 13 '25 16:10 ferponcem

👋 @ferponcem 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.

github-actions[bot] avatar Oct 13 '25 18:10 github-actions[bot]

@ferponcem let me know when you are done for today on this: I will do a git merge of main to get a cleaner diff

Remi-Gau avatar Oct 24 '25 14:10 Remi-Gau

Codecov Report

:x: Patch coverage is 8.51064% with 43 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 35.52%. Comparing base (cf4d056) to head (dc45fed).

Files with missing lines Patch % Lines
nilearn/glm/first_level/first_level.py 8.51% 43 Missing :warning:

:exclamation: There is a different number of reports uploaded between BASE (cf4d056) and HEAD (dc45fed). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (cf4d056) HEAD (dc45fed)
macos-latest_3.11_plotting 1 0
macos-latest_3.13_plotting 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5761       +/-   ##
===========================================
- Coverage   95.50%   35.52%   -59.98%     
===========================================
  Files         307      307               
  Lines       41420    41466       +46     
  Branches     4218     4222        +4     
===========================================
- Hits        39559    14732    -24827     
- Misses       1176    25898    +24722     
- Partials      685      836      +151     
Flag Coverage Δ
macos-latest_3.10_min 31.67% <8.51%> (?)
macos-latest_3.10_plot_min 31.66% <8.51%> (?)
macos-latest_3.10_plotting 31.69% <8.51%> (?)
macos-latest_3.11_plotting ?
macos-latest_3.13_plotting ?
macos-latest_3.14_plotting 31.67% <8.51%> (?)
ubuntu-latest_3.10_plotting 31.69% <8.51%> (?)
ubuntu-latest_3.10_pytest_mpl 16.80% <8.51%> (-0.02%) :arrow_down:
ubuntu-latest_3.14_plotting 31.67% <8.51%> (?)
ubuntu-latest_3.14_pre 31.67% <8.51%> (?)
windows-latest_3.10_plotting 31.68% <8.51%> (?)
windows-latest_3.14_plotting 31.67% <8.51%> (?)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Oct 24 '25 15:10 codecov[bot]

I added a bunch of comments as TODO in the top message of the PR: ask me if you need clarifications

Remi-Gau avatar Oct 27 '25 07:10 Remi-Gau