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

I am not sure if I should keep definition of self.region_id_name in __init__ or move it to fit after definition of self.labels_img_

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.03%. Comparing base (abb80ff) to head (8b03f95).
Report is 65 commits behind head on main.

@@            Coverage Diff             @@
##             main    #4360      +/-   ##
==========================================
+ Coverage   91.85%   92.03%   +0.17%     
==========================================
  Files         144      143       -1     
  Lines       16419    16639     +220     
  Branches     3434     3533      +99     
==========================================
+ Hits        15082    15313     +231     
+ Misses        792      758      -34     
- Partials      545      568      +23     

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

This seems top break the report builder script in the doc build:

Traceback (most recent call last):
  File "/home/runner/work/nilearn/nilearn/doc/visual_testing/reporter_visual_inspection_suite.py", line 438, in <module>
    report_nifti_labels_masker()
  File "/home/runner/work/nilearn/nilearn/doc/visual_testing/reporter_visual_inspection_suite.py", line 313, in report_nifti_labels_masker
    masker = NiftiLabelsMasker(
             ^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/nilearn/maskers/nifti_labels_masker.py", line 212, in __init__
    _utils.niimg.safe_get_data(labels_img)
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/nilearn/_utils/niimg.py", line 58, in safe_get_data
    data = _get_data(img)
           ^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/nilearn/_utils/niimg.py", line 23, in _get_data
    if img._data_cache is not None:
       ^^^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute '_data_cache'

Can you try to run doc/visual_testing/reporter_visual_inspection_suite.py to see what's going on?

Do I need to do anything regarding this PR? I think all the issues are addressed.

Since this is a new public attribute, do you think you could update the doc string to explain what it is?

I'm not sure if we want region_id_name_ to be a public attribute, do we? My goal was to create so we can use it to create region_names_

@Remi-Gau but I think it's a good idea to add some comments to explain to the developers what this attribute is

Since this is a new public attribute, do you think you could update the doc string to explain what it is?

I'm not sure if we want region_id_name_ to be a public attribute, do we? My goal was to create so we can use it to create region_names_

OK in this case rename it to _region_id_name to signify this a "private" attribute.