Skip to content

Conversation

@tharu-jwd
Copy link
Contributor

Summary

Fixes inconsistency between the plot_evoked_joint function signature and its docstring regarding the exclude parameter default value.

Problem

  • evoked.plot_joint() method uses exclude='bads' as default
  • plot_evoked_joint() function had exclude=None as default but docstring claimed it defaults to None
  • This created API documentation inconsistency

Solution

  • Changed plot_evoked_joint default parameter from exclude=None to exclude='bads'
  • Updated docstring to reflect the new default

Testing

  • Verified existing tests still pass
  • Confirmed API consistency between functions
  • Tested backward compatibility with various exclude options
  • Validated pre-commit hooks pass

Fixes #13157

The docstring for plot_evoked_joint stated that the exclude parameter
defaults to None, but the actual default was inconsistent with the
evoked.plot_joint method which uses exclude="bads" as default.

This commit fixes the inconsistency by:
- Changing plot_evoked_joint default from exclude=None to exclude="bads"
- Updating the docstring to reflect the new default

Fixes mne-tools#13157
@welcome
Copy link

welcome bot commented Aug 22, 2025

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

axes are passed make sure to set ``title=None``, otherwise some of your
axes may be removed during placement of the title axis.
%(picks_all)s
exclude : None | list of str | 'bads'
Copy link
Member

Choose a reason for hiding this comment

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

the code in #13157 shows that None isn't even valid input, so we should remove it here too:

Suggested change
exclude : None | list of str | 'bads'
exclude : list of str | 'bads'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

the filename of this file needs the PR number, not the issue number that it addresses. So change to 13391.bugfix.rst

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Please Check.

- Remove 'None' from exclude parameter type in docstring
- Rename changelog file to use PR number (13391.bugfix.rst)
@drammock drammock enabled auto-merge (squash) August 22, 2025 16:32
@drammock drammock merged commit 0b374c1 into mne-tools:main Aug 22, 2025
32 checks passed
@welcome
Copy link

welcome bot commented Aug 22, 2025

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

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.

evoked.plot_joint API exclude parameter docstring is inconsistent with the code

2 participants