[MAINT] make `all` and API doc more consistent #4157

Remi-Gau · 2023-12-13T08:45:08Z

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

Modules should explicitly declare the names in their public API using the __all__ attribute but that we have some things in the __all__ that is not in the API doc.

This PR fixes some issues:

removes functions that are in the public API of some modules but not in the doc
orders __all__ alphabetically to make this comparison easier in the future

github-actions · 2023-12-13T08:45:22Z

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

Remi-Gau · 2023-12-13T08:45:37Z

nilearn/glm/first_level/__init__.py

@@ -2,7 +2,6 @@
    check_design_matrix,
    make_first_level_design_matrix,
 )
-from nilearn.glm.first_level.experimental_paradigm import check_events


check_events not in API doc

Remi-Gau · 2023-12-13T08:46:46Z

nilearn/image/__init__.py

+# TODO move copy_img out of nilearn._utils..niimg
 from .._utils.niimg import copy_img
+
+# TODO move concat_niimgs out of nilearn._utils.niimg_conversions
 from .._utils.niimg_conversions import concat_niimgs as concat_imgs


Tempted to say that things that are part of the user facing API should NOT be in a part of the package that is private.

Remi-Gau · 2023-12-13T08:47:12Z

nilearn/maskers/__init__.py

@@ -1,6 +1,5 @@
 """The :mod:`nilearn.maskers` contains masker objects."""

-from ._utils import compute_middle_image


compute_middle_image not in the API doc

nilearn/plotting/__init__.py

Remi-Gau · 2023-12-13T08:52:38Z

nilearn/reporting/__init__.py

+# TODO consider making _get_clusters_table module public?
 from nilearn.reporting._get_clusters_table import get_clusters_table


User facing function comes from a private module

codecov · 2023-12-13T09:53:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (af37456) 91.71% compared to head (f9f2ee0) 91.92%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4157      +/-   ##
==========================================
+ Coverage   91.71%   91.92%   +0.20%     
==========================================
  Files         145      145              
  Lines       16337    16337              
  Branches     3407     3407              
==========================================
+ Hits        14983    15017      +34     
+ Misses        810      777      -33     
+ Partials      544      543       -1

Flag	Coverage Δ
macos-latest_3.10_test_plotting	`91.69% <100.00%> (ø)`
macos-latest_3.11_test_plotting	`91.69% <100.00%> (ø)`
macos-latest_3.12_test_plotting	`91.69% <100.00%> (ø)`
macos-latest_3.8_test_plotting	`91.66% <100.00%> (ø)`
macos-latest_3.9_test_plotting	`91.66% <100.00%> (ø)`
ubuntu-latest_3.10_test_plotting	`91.69% <100.00%> (ø)`
ubuntu-latest_3.11_test_plotting	`91.69% <100.00%> (ø)`
ubuntu-latest_3.12_test_plotting	`91.69% <100.00%> (ø)`
ubuntu-latest_3.12_test_pre	`91.69% <100.00%> (ø)`
ubuntu-latest_3.8_test_min	`68.86% <100.00%> (?)`
ubuntu-latest_3.8_test_plot_min	`91.42% <100.00%> (?)`
ubuntu-latest_3.8_test_plotting	`91.66% <100.00%> (ø)`
ubuntu-latest_3.9_test_plotting	`91.66% <100.00%> (ø)`
windows-latest_3.10_test_plotting	`91.67% <100.00%> (ø)`
windows-latest_3.11_test_plotting	`91.67% <100.00%> (ø)`
windows-latest_3.12_test_plotting	`91.67% <100.00%> (ø)`
windows-latest_3.8_test_plotting	`91.63% <100.00%> (ø)`
windows-latest_3.9_test_plotting	`91.64% <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.

Remi-Gau · 2023-12-13T10:30:02Z

nilearn/masking.py

+]
+
+
+class _MaskWarning(UserWarning):


Warning not used outside of this module

Remi-Gau · 2023-12-13T10:30:42Z

nilearn/glm/__init__.py

-    "first_level",
-    "second_level",


note that those were duplicated.

ymzayek

I think you just need to adjust the number in test_number_public_classes

Remi-Gau · 2023-12-13T11:42:50Z

I think you just need to adjust the number in test_number_public_classes

thanks!!! 🤗

should be fixed now

bthirion

LGTM, thx

add comments and some fixes to make __all__ and API doc consistent

63c96a9

Remi-Gau commented Dec 13, 2023

View reviewed changes

nilearn/plotting/__init__.py Show resolved Hide resolved

Remi-Gau commented Dec 13, 2023

View reviewed changes

fix import

65f8a7f

reorder __all__ alphabetically

731d418

Remi-Gau commented Dec 13, 2023

View reviewed changes

nilearn/masking.py

]

class _MaskWarning(UserWarning):

Copy link

Collaborator Author

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning not used outside of this module

Remi-Gau commented Dec 13, 2023

View reviewed changes

nilearn/glm/__init__.py

Comment on lines -27 to -28

"first_level",

"second_level",

Copy link

Collaborator Author

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that those were duplicated.

Remi-Gau mentioned this pull request Dec 13, 2023

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

Closed

remove TODOs

7b0d102

Remi-Gau changed the title ~~[WIP] DO NOT REVIEW - make __all__ and API doc consistent~~ [MAINT] make __all__ and API doc more consistent Dec 13, 2023

Remi-Gau marked this pull request as ready for review December 13, 2023 10:45

ymzayek approved these changes Dec 13, 2023

View reviewed changes

ymzayek reviewed Dec 13, 2023

View reviewed changes

fix number of public classes

f9f2ee0

bthirion approved these changes Dec 13, 2023

View reviewed changes

Remi-Gau merged commit 9362f97 into nilearn:main Dec 13, 2023
32 checks passed

Remi-Gau deleted the public_al_init branch December 13, 2023 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MAINT] make `all` and API doc more consistent #4157

[MAINT] make `all` and API doc more consistent #4157

Remi-Gau commented Dec 13, 2023 •

edited

github-actions bot commented Dec 13, 2023

Remi-Gau Dec 13, 2023

Remi-Gau Dec 13, 2023

Remi-Gau Dec 13, 2023

Remi-Gau Dec 13, 2023

codecov bot commented Dec 13, 2023 •

edited

Remi-Gau Dec 13, 2023

Remi-Gau Dec 13, 2023

ymzayek left a comment

Remi-Gau commented Dec 13, 2023

bthirion left a comment

		@@ -1,6 +1,5 @@
		"""The :mod:`nilearn.maskers` contains masker objects."""

		from ._utils import compute_middle_image

		# TODO consider making _get_clusters_table module public?
		from nilearn.reporting._get_clusters_table import get_clusters_table

[MAINT] make __all__ and API doc more consistent #4157

[MAINT] make __all__ and API doc more consistent #4157

Conversation

Remi-Gau commented Dec 13, 2023 • edited

github-actions bot commented Dec 13, 2023

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

codecov bot commented Dec 13, 2023 • edited

Codecov Report

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

Remi-Gau Dec 13, 2023

Choose a reason for hiding this comment

ymzayek left a comment

Choose a reason for hiding this comment

Remi-Gau commented Dec 13, 2023

bthirion left a comment

Choose a reason for hiding this comment

[MAINT] make `all` and API doc more consistent #4157

[MAINT] make `all` and API doc more consistent #4157

Remi-Gau commented Dec 13, 2023 •

edited

codecov bot commented Dec 13, 2023 •

edited