Skip to content

[DOC] update cut_coords in examples #5913

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.

Sign up for GitHub

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

Jump to bottom
Merged
Remi-Gau merged 1 commit into from
Dec 19, 2025
Merged

[DOC] update cut_coords in examples #5913

Remi-Gau merged 1 commit into from
Dec 19, 2025
+1 −1

Conversation

@Remi-Gau
Copy link
Collaborator

@Remi-Gau Remi-Gau commented Dec 19, 2025

  • Closes none but should fix doc build on main

Changes proposed in this pull request:

  • range is not accepted as cut_coord

@github-actions
Copy link
Contributor

github-actions bot commented Dec 19, 2025

👋 @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 commented Dec 19, 2025
View reviewed changes
examples/01_plotting/plot_3d_map_to_surface_projection.py
display_mode="x",
threshold=1.0,
cut_coords=range(0, 51, 10),
cut_coords=list(range(0, 51, 10)),
Copy link
Collaborator Author

@Remi-Gau Remi-Gau Dec 19, 2025

Choose a reason for hiding this comment

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

I wonder if we should allow iterator and generator to avoid a regression like this one

Copy link
Collaborator

@hndgzkn hndgzkn Dec 19, 2025

Choose a reason for hiding this comment

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

I think it depends. Iterator or generator was not mentioned in the doc as far as I remember. So I followed the specification in docstring to restrict the types. Is it likely/necessary to specify an iterator?

I would actually be in favor of limiting types of parameters unless necessary. When we enlarge possible parameter types, that requires more checks.

Copy link
Collaborator Author

@Remi-Gau Remi-Gau Dec 19, 2025

Choose a reason for hiding this comment

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

agreed so let's leave things like this for now

@Remi-Gau Remi-Gau marked this pull request as ready for review December 19, 2025 16:09
@Remi-Gau Remi-Gau merged commit 60605de into nilearn:main Dec 19, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hndgzkn hndgzkn hndgzkn left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Remi-Gau @hndgzkn