[MAINT] move user facing functions out of private modules #4166

Remi-Gau · 2023-12-16T10:24:15Z

Relates to making the content of __all__ consistent with the API references in the docs. #4137 (comment)
Relates to Check for inconsistent naming of functions according to PEP8 convention #3319

Changes proposed in this pull request:

make _get_clusters_table module public
move copy_img out of nilearn._utils.niimg

github-actions · 2023-12-16T10:24:28Z

👋 @Remi-Gau 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-12-16T14:19:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (434f0be) 92.00% compared to head (52ff9cb) 92.02%.
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4166      +/-   ##
==========================================
+ Coverage   92.00%   92.02%   +0.01%     
==========================================
  Files         145      144       -1     
  Lines       16378    16374       -4     
  Branches     3431     3427       -4     
==========================================
- Hits        15069    15068       -1     
+ Misses        768      766       -2     
+ Partials      541      540       -1

Flag	Coverage Δ
macos-latest_3.10_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
macos-latest_3.11_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
macos-latest_3.12_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
macos-latest_3.8_test_plotting	`91.76% <100.00%> (+0.01%)`	⬆️
macos-latest_3.9_test_plotting	`91.77% <100.00%> (?)`
ubuntu-latest_3.10_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.11_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.12_test_plotting	`91.80% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.12_test_pre	`91.80% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.8_test_min	`68.98% <90.00%> (+<0.01%)`	⬆️
ubuntu-latest_3.8_test_plot_min	`91.52% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.8_test_plotting	`91.76% <100.00%> (+0.01%)`	⬆️
ubuntu-latest_3.9_test_plotting	`91.77% <100.00%> (+0.01%)`	⬆️
windows-latest_3.10_test_plotting	`91.78% <100.00%> (+0.01%)`	⬆️
windows-latest_3.11_test_plotting	`91.78% <100.00%> (+0.01%)`	⬆️
windows-latest_3.12_test_plotting	`91.78% <100.00%> (+0.01%)`	⬆️
windows-latest_3.8_test_plotting	`91.74% <100.00%> (+0.01%)`	⬆️
windows-latest_3.9_test_plotting	`91.74% <100.00%> (+0.01%)`	⬆️

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

LGTM, aside codecov complaints

Remi-Gau · 2023-12-18T08:58:08Z

will add a test for the deprecation warning to help the coverage

Remi-Gau · 2023-12-20T12:58:09Z

@bthirion for this PR do you think that this should be mentioned in the changelog like in the previous PRs

nilearn/doc/changes/latest.rst

Lines 48 to 50 in 8b0657b

    
           - :bdg-dark:`Code` Remove leading underscore from non private functions to align with PEP8 (:gh:`4086` by `Rémi Gau`_). 
        
           - :bdg-dark:`Code` Make ``decoding/proximal_operator`` explicitly private to align with PEP8 (:gh:`4153` by `Rémi Gau`_). 
        
           - :bdg-dark:`Code` Make private functions public when used outside of their module ``nilearn.interface`` to align with PEP8 (:gh:`4168` by `Rémi Gau`_).

Technically users should not be importing from those private modules and there is a warning if they do but I think it can be nice to mention it in the changelog

same comment for PR [MAINT] move user facing functions out of private modules - part 2 #4167

bthirion · 2023-12-20T23:01:31Z

Yes, put it in the changelog.
Thx !

nilearn/_utils/niimg.py

ymzayek · 2024-01-04T09:39:12Z

nilearn/_utils/tests/test_niimg.py

+def test_copy_warning(img_3d_zeros_eye):
+    with pytest.warns(
+        DeprecationWarning,
+        match="Import this function from 'nilearn.image.copy_img'",
+    ):
+        niimg.copy_img(img_3d_zeros_eye)


LGTM you just forgot to remove this test

If I had to pay for every extra ci workflows that has to run because of my absent mindedness... I'd be coding from under a bridge most of the time...

bthirion

LGTM, aside the test to be removed. Thx !

Remi-Gau added 3 commits December 16, 2023 10:46

make _get_clusters_tables public

9c6844d

mv copy_img

b53a29a

lint

1c5ae2e

Remi-Gau added 2 commits December 16, 2023 12:09

update number of public functions

0c71eb8

use fixture

a1f06f9

bthirion reviewed Dec 18, 2023

View reviewed changes

Remi-Gau added 2 commits December 18, 2023 12:50

add test for warning

fbf4254

Merge branch 'main' into make_func_public

f231805

Remi-Gau added 2 commits December 21, 2023 10:32

Merge remote-tracking branch 'upstream/main' into make_func_public

43f0fe8

update changelog

6817e1c

ymzayek reviewed Jan 2, 2024

View reviewed changes

nilearn/_utils/niimg.py Outdated Show resolved Hide resolved

Remi-Gau mentioned this pull request Jan 3, 2024

[MAINT] move user facing functions out of private modules - part 2 #4167

Merged

rm copy_imgs from private module

84ec7ea

Remi-Gau requested review from ymzayek and bthirion January 4, 2024 08:23

Remi-Gau mentioned this pull request Jan 4, 2024

making the content of __all__ consistent with the API references in the docs. #4137

Closed

ymzayek reviewed Jan 4, 2024

View reviewed changes

bthirion approved these changes Jan 4, 2024

View reviewed changes

rm test

52ff9cb

Remi-Gau merged commit 1374eee into nilearn:main Jan 5, 2024
32 checks passed

Remi-Gau deleted the make_func_public branch January 5, 2024 14:33

Remi-Gau mentioned this pull request Feb 2, 2024

nilearn 0.10.3 breaks nimare neurostuff/NiMARE#872

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINT] move user facing functions out of private modules #4166

[MAINT] move user facing functions out of private modules #4166

Remi-Gau commented Dec 16, 2023 •

edited

github-actions bot commented Dec 16, 2023

codecov bot commented Dec 16, 2023 •

edited

bthirion left a comment

Remi-Gau commented Dec 18, 2023

Remi-Gau commented Dec 20, 2023

bthirion commented Dec 20, 2023

ymzayek Jan 4, 2024

Remi-Gau Jan 4, 2024 •

edited

bthirion left a comment

[MAINT] move user facing functions out of private modules #4166

[MAINT] move user facing functions out of private modules #4166

Conversation

Remi-Gau commented Dec 16, 2023 • edited

github-actions bot commented Dec 16, 2023

codecov bot commented Dec 16, 2023 • edited

Codecov Report

bthirion left a comment

Choose a reason for hiding this comment

Remi-Gau commented Dec 18, 2023

Remi-Gau commented Dec 20, 2023

bthirion commented Dec 20, 2023

ymzayek Jan 4, 2024

Choose a reason for hiding this comment

Remi-Gau Jan 4, 2024 • edited

Choose a reason for hiding this comment

bthirion left a comment

Choose a reason for hiding this comment

Remi-Gau commented Dec 16, 2023 •

edited

codecov bot commented Dec 16, 2023 •

edited

Remi-Gau Jan 4, 2024 •

edited