DOC: Remove invalid ndarray option from colormap docstrings#13797
DOC: Remove invalid ndarray option from colormap docstrings#13797Gnefil wants to merge 3 commits intomne-tools:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates MNE-Python documentation to match the actual accepted types for the colormap parameter (fixing a mismatch where docs claimed np.ndarray was allowed, but runtime validation rejects it), and adds new-contributor attribution for the changelog.
Changes:
- Update the shared
colormapparameter docstring template to removenp.ndarrayand document MatplotlibColormapobjects instead. - Add a Towncrier dev changelog entry describing the documentation fix.
- Add the contributor name/link to
doc/changes/names.inc.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
mne/utils/docs.py |
Adjusts the canonical colormap parameter doc snippet to reflect accepted types. |
doc/changes/dev/13797.other.rst |
Adds a changelog entry for the documentation correction. |
doc/changes/names.inc |
Registers the new contributor for changelog attribution. |
|
Hi Gnefil, thank you for your contribution. If you sign up for a free account on circleci and link it to your github repo, the workflows related to documentation building will run (they are currently failing). |
|
Hii @CarinaFo, thanks for replying!
I have successfully signed up for CircleCI, however I am stucked in "No CircleCI organizations found" and "Join via invite". Account integration doesn't show GitHub, so I presume it is linked. This is after Join via GitHub option after signing up. Could you clarify what should be the next steps? Thanks in advance! |
JiwaniZakir
left a comment
There was a problem hiding this comment.
The docdict["colormap"] entry in mne/utils/docs.py is a shared docstring used across multiple functions, not just plot_source_estimates — so the changelog entry in 13797.other.rst likely understates the scope of this change. It would be worth auditing all callers of this docdict key (e.g., with a grep for "colormap" in the codebase) to confirm they all accept matplotlib.colors.Colormap and no longer accept raw np.ndarray inputs.
More critically, this PR updates documentation without a corresponding code change to the underlying implementation. If any of the functions that use this docstring still accept and handle np.ndarray arguments internally (as the old docstring described), the new docstring is now inaccurate and will silently mislead users who pass arrays — a behavioral regression in documentation fidelity. Conversely, if ndarray support was already broken or unimplemented, a deprecation notice or explicit error with a clear message would be more user-friendly than silently dropping it from the docs.
Also, 13797.other.rst is missing a newline at end of file, which may cause formatting issues in the rendered changelog.
|
Hii Zakir, Thanks for your critical comments!
I see, then I am going to audit colormap callers in the whole codebase to see whether updating
I beg to politely differ, aligning documentation with the actual behaviour should be the priority, then, raising an extra error for a type that is not accepted seems redundant. Anyways, I would check the previous step to see what action is suitable.
Got it, will fix it in the next commit. Thanks again for your extensive thoughts, really appreciate it! |
@JiwaniZakir we don't need new line in changelog entry |
Reference issue
Fixes #13189 .
What does this implement/fix?
This PR solves the conflict between the documentation and the actual signature of
mne.viz.plot_source_estimates, which doesn't accept a numpy array for colormap.Additional information
A possible future minor enhancement would be to include a numpy array option and transform to colormap using matplotlib as suggested by larsoner in comments. If agreed, that could be my next PR.
Additionally, this is my first time contributing. I looked at the contributing guidelines, played with the code, and viewed how others create issues and PRs. But still, I might miss something, so don't hesitate to point out any mistakes! Finally, this is a work by Lifeng, a 2026 GSoC applicant for project 2.