[DOC] Add docstrings for some bigger private functions in `load_confounds.` #3863

htwangtw · 2023-07-21T14:05:33Z

Closes #3826.

Focus on some wrapper functions that I have seen used incorrectly from user submitted issues.
With the help of copilot.

With the help of copilot.

github-actions · 2023-07-21T16:18:25Z

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

codecov · 2023-07-21T16:33:08Z

Codecov Report

Merging #3863 (87f03e4) into main (e002d24) will increase coverage by 0.01%.
Report is 31 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3863      +/-   ##
==========================================
+ Coverage   91.59%   91.60%   +0.01%     
==========================================
  Files         134      134              
  Lines       15694    15751      +57     
  Branches     3270     3281      +11     
==========================================
+ Hits        14375    14429      +54     
- Misses        770      772       +2     
- Partials      549      550       +1

Flag	Coverage Δ
macos-latest_3.10	`91.53% <100.00%> (+0.01%)`	⬆️
macos-latest_3.11	`91.53% <100.00%> (+0.01%)`	⬆️
macos-latest_3.8	`91.49% <100.00%> (+0.01%)`	⬆️
macos-latest_3.9	`91.49% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.10	`91.53% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.11	`91.53% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.8	`91.49% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.9	`91.49% <100.00%> (+0.01%)`	⬆️
windows-latest_3.10	`91.47% <100.00%> (+0.01%)`	⬆️
windows-latest_3.11	`91.47% <100.00%> (+0.01%)`	⬆️
windows-latest_3.8	`91.43% <100.00%> (+0.01%)`	⬆️
windows-latest_3.9	`91.43% <100.00%> (+0.01%)`	⬆️

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

Files Changed	Coverage Δ
nilearn/interfaces/fmriprep/load_confounds.py	`100.00% <ø> (ø)`
...earn/interfaces/fmriprep/load_confounds_compcor.py	`100.00% <ø> (ø)`
...n/interfaces/fmriprep/load_confounds_components.py	`100.00% <ø> (ø)`
...ilearn/interfaces/fmriprep/load_confounds_scrub.py	`100.00% <ø> (ø)`
...arn/interfaces/fmriprep/load_confounds_strategy.py	`100.00% <ø> (ø)`
...ilearn/interfaces/fmriprep/load_confounds_utils.py	`100.00% <100.00%> (ø)`

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

nilearn/interfaces/fmriprep/load_confounds.py

bthirion

Thx !
Maybe we could take this opportunity to improve docstrings quality...

nilearn/interfaces/fmriprep/load_confounds.py

Co-authored-by: bthirion <bertrand.thirion@inria.fr>

…tw/nilearn into load_confounds_private_docstrings

bthirion

Thx. A few changes have been missed. Best,

nilearn/interfaces/fmriprep/load_confounds_compcor.py

ymzayek

@htwangtw thanks for adding the docstrings to these private functions! Most of my comments actually have to do with modified docstrings of public functions but I think it's fine to change them in this PR. If you think it's a bigger job (I didn't check parameters that aren't modified) and wish to leave it for another PR that's fine too

nilearn/interfaces/fmriprep/load_confounds.py

nilearn/interfaces/fmriprep/load_confounds_strategy.py

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

Co-authored-by: bthirion <bertrand.thirion@inria.fr>

- remove outdated one-hot encoder related description - remove options from parameter descriptions, replaced with default option - remove redundant default option descriptions in the docstring

co-authored by GitHub Co-pilot.

ymzayek

Thx @htwangtw , just a few minor things

nilearn/interfaces/fmriprep/load_confounds_components.py

nilearn/interfaces/fmriprep/load_confounds_utils.py

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

ymzayek

Thx!

Remi-Gau

Good with me

thanks @htwangtw

Add docstrings for some bigger private functions

cc0f05e

With the help of copilot.

htwangtw marked this pull request as ready for review July 21, 2023 17:19

Remi-Gau reviewed Jul 22, 2023

View reviewed changes

nilearn/interfaces/fmriprep/load_confounds.py Outdated Show resolved Hide resolved

bthirion reviewed Jul 23, 2023

View reviewed changes

nilearn/interfaces/fmriprep/load_confounds.py Outdated Show resolved Hide resolved

nilearn/interfaces/fmriprep/load_confounds.py Outdated Show resolved Hide resolved

nilearn/interfaces/fmriprep/load_confounds.py Outdated Show resolved Hide resolved

htwangtw and others added 4 commits August 3, 2023 11:22

Apply suggestions from code review

95b2a73

Co-authored-by: bthirion <bertrand.thirion@inria.fr>

Add cross-reference links to type definitions

706c4a7

Docstring for compcor related private function

92bba5e

Merge branch 'load_confounds_private_docstrings' of github.com:htwang…

a744878

…tw/nilearn into load_confounds_private_docstrings

bthirion reviewed Aug 4, 2023

View reviewed changes

ymzayek reviewed Aug 4, 2023

View reviewed changes

htwangtw and others added 3 commits August 4, 2023 13:15

Apply suggestions from code review

54d333e

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

Apply suggestions from code review

bdd8885

Co-authored-by: bthirion <bertrand.thirion@inria.fr>

Simplify docstring

b0ea1cb

- remove outdated one-hot encoder related description - remove options from parameter descriptions, replaced with default option - remove redundant default option descriptions in the docstring

htwangtw mentioned this pull request Aug 4, 2023

Simplify load_confounds API: n_compcor #3886

Open

htwangtw added 2 commits August 7, 2023 13:47

omissed out the default value of scrubbing

c7cb3c9

Complete all the docstring

fc2da61

co-authored by GitHub Co-pilot.

htwangtw requested review from ymzayek and Remi-Gau August 8, 2023 20:10

ymzayek reviewed Aug 9, 2023

View reviewed changes

Apply suggestions from code review

87f03e4

Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>

ymzayek approved these changes Aug 10, 2023

View reviewed changes

Remi-Gau approved these changes Aug 10, 2023

View reviewed changes

Remi-Gau merged commit ab8eae6 into nilearn:main Aug 10, 2023
29 checks passed

htwangtw deleted the load_confounds_private_docstrings branch August 15, 2023 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DOC] Add docstrings for some bigger private functions in `load_confounds.` #3863

[DOC] Add docstrings for some bigger private functions in `load_confounds.` #3863

htwangtw commented Jul 21, 2023 •

edited

github-actions bot commented Jul 21, 2023 •

edited by Remi-Gau

codecov bot commented Jul 21, 2023 •

edited

bthirion left a comment

bthirion left a comment

ymzayek left a comment

ymzayek left a comment

ymzayek left a comment

Remi-Gau left a comment

[DOC] Add docstrings for some bigger private functions in load_confounds. #3863

[DOC] Add docstrings for some bigger private functions in load_confounds. #3863

Conversation

htwangtw commented Jul 21, 2023 • edited

github-actions bot commented Jul 21, 2023 • edited by Remi-Gau

codecov bot commented Jul 21, 2023 • edited

Codecov Report

bthirion left a comment

Choose a reason for hiding this comment

bthirion left a comment

Choose a reason for hiding this comment

ymzayek left a comment

Choose a reason for hiding this comment

ymzayek left a comment

Choose a reason for hiding this comment

ymzayek left a comment

Choose a reason for hiding this comment

Remi-Gau left a comment

Choose a reason for hiding this comment

[DOC] Add docstrings for some bigger private functions in `load_confounds.` #3863

[DOC] Add docstrings for some bigger private functions in `load_confounds.` #3863

htwangtw commented Jul 21, 2023 •

edited

github-actions bot commented Jul 21, 2023 •

edited by Remi-Gau

codecov bot commented Jul 21, 2023 •

edited