Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] update dataset descriptions #4215

Merged
merged 16 commits into from Jan 17, 2024
Merged

Conversation

Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Jan 12, 2024

Changes proposed in this pull request:

  • add failing tests to show missing descriptions
  • add dataset descriptions and make sure they are returned by all fetchers (except the openneuro one as the description could be different for every dataset)

unrelated: update some of the doc string in datasets.func.py

Copy link
Contributor

github-actions bot commented Jan 12, 2024

👋 @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 Remi-Gau changed the title [WIP][DOC] add failing tests for missing descriptions [WIP][DOC] add missing dataset descriptions Jan 12, 2024
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (3d80bac) 92.10% compared to head (a98e1c4) 92.07%.

Files Patch % Lines
nilearn/datasets/func.py 59.09% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4215      +/-   ##
==========================================
- Coverage   92.10%   92.07%   -0.03%     
==========================================
  Files         144      144              
  Lines       16368    16385      +17     
  Branches     3427     3427              
==========================================
+ Hits        15075    15086      +11     
- Misses        754      760       +6     
  Partials      539      539              
Flag Coverage Δ
macos-latest_3.10_test_plotting 91.86% <65.38%> (-0.03%) ⬇️
macos-latest_3.11_test_plotting 91.86% <65.38%> (-0.03%) ⬇️
macos-latest_3.12_test_plotting 91.86% <65.38%> (-0.03%) ⬇️
macos-latest_3.8_test_plotting 91.82% <65.38%> (-0.03%) ⬇️
macos-latest_3.9_test_plotting 91.83% <65.38%> (-0.03%) ⬇️
ubuntu-latest_3.10_test_plotting 91.86% <65.38%> (-0.03%) ⬇️
ubuntu-latest_3.11_test_plotting 91.86% <65.38%> (-0.03%) ⬇️
ubuntu-latest_3.12_test_plotting ?
ubuntu-latest_3.12_test_pre ?
ubuntu-latest_3.8_test_min 68.97% <65.38%> (-0.01%) ⬇️
ubuntu-latest_3.8_test_plot_min 91.57% <65.38%> (-0.03%) ⬇️
ubuntu-latest_3.8_test_plotting 91.82% <65.38%> (-0.03%) ⬇️
ubuntu-latest_3.9_test_plotting 91.83% <65.38%> (-0.03%) ⬇️
windows-latest_3.10_test_plotting 91.84% <65.38%> (-0.03%) ⬇️
windows-latest_3.11_test_plotting 91.84% <65.38%> (-0.03%) ⬇️
windows-latest_3.12_test_plotting 91.84% <65.38%> (-0.03%) ⬇️
windows-latest_3.8_test_plotting ?
windows-latest_3.9_test_plotting ?

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 Remi-Gau changed the title [WIP][DOC] add missing dataset descriptions [WIP][DOC] update dataset descriptions Jan 12, 2024
nilearn/datasets/func.py Outdated Show resolved Hide resolved
for f in files
]
return data_dir, sorted(file_list)
description = get_dataset_descr("language_localizer_demo")
return data_dir, sorted(file_list), description
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Remi-Gau
Copy link
Collaborator Author

Remi-Gau commented Jan 12, 2024

TODOs for other PRs:

  • make sure all dataset fetchers return a Bunch that include a description
  • centralize dataset / atlas description contained to the description RST file:
    • in doc string (if there is) of the fetcher
    • in doc strings of the examples that use the fetcher
  • print the description of the dataset in the examples that use the fetcher
  • render the description RST files as part of the doc
  • make sure that articles mentioned in the descriptions are part of our bibliography

@@ -0,0 +1,34 @@
localizer first level
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dataset hosted here: https://osf.io/v7hsw/?view_only=

but cannot find a whole lot more info about it

@Remi-Gau
Copy link
Collaborator Author

Looking at the data from those 2 datasets, I am not sure I can tell the difference between the 2?

Events files look kind of similar.

  • fetch_bids_langloc_dataset
  • fetch_language_localizer_demo_dataset

Also the bids_langloc_dataset seems unused in our examples.

doc/changes/latest.rst Outdated Show resolved Hide resolved
examples/07_advanced/plot_beta_series.py Outdated Show resolved Hide resolved
@Remi-Gau Remi-Gau changed the title [WIP][DOC] update dataset descriptions [DOC] update dataset descriptions Jan 16, 2024
@Remi-Gau Remi-Gau marked this pull request as ready for review January 16, 2024 09:18
nilearn/datasets/description/spm_auditory.rst Outdated Show resolved Hide resolved
nilearn/datasets/func.py Outdated Show resolved Hide resolved
doc/changes/latest.rst Outdated Show resolved Hide resolved
Remi-Gau and others added 2 commits January 16, 2024 11:28
Co-authored-by: Yasmin <63292494+ymzayek@users.noreply.github.com>
Copy link
Member

@ymzayek ymzayek left a comment

Choose a reason for hiding this comment

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

The changes look good thanks!

Copy link
Member

@bthirion bthirion left a comment

Choose a reason for hiding this comment

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

LGTM, thx for all the cleaning.

@Remi-Gau Remi-Gau merged commit 2a8e184 into nilearn:main Jan 17, 2024
31 of 32 checks passed
@Remi-Gau Remi-Gau deleted the dataset_descriptions branch January 17, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] missing description of several datasets
3 participants