Skip to content

Conversation

@mcvain
Copy link
Contributor

@mcvain mcvain commented Feb 10, 2023

Reference issue

Example: Fixes #11460

What does this implement/fix?

  • Fixes the ValueError when passing in an object-type ndarray into the ch_groups argument of mne.viz.plot_sensors()
  • Added array_like to acceptable types and added an explanation for creating unequal-sized channel groups in the documentation

@welcome
Copy link

welcome bot commented Feb 10, 2023

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

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Looks like a good start! Can you

  1. Add a line to doc/changes/latest.inc using the :newcontrib: role, probably at the top of the enhancements section about this doc update?
  2. Add your name to doc/changes/names.inc
  3. Add a tiny test that actually uses the list-of-list functionality -- I'm guessing we don't have one yet?

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Just pushed a tiny commit to adjust the type suggestion in the docstring, marking for merge when green. Thanks in advance @mcvain !

@larsoner larsoner enabled auto-merge (squash) February 13, 2023 16:12
@larsoner larsoner merged commit 3e69d02 into mne-tools:main Feb 13, 2023
@welcome
Copy link

welcome bot commented Feb 13, 2023

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

larsoner added a commit to drammock/mne-python that referenced this pull request Feb 22, 2023
* upstream/main: (46 commits)
  Fix docstrings by replacing str with path-like and fix double backticks for formatting (mne-tools#11499)
  Use pathlib.Path instead of os.path to handle files and folders [circle deploy] (mne-tools#11473)
  MAINT: Fix Circle [circle deploy] (mne-tools#11497)
  MAINT: Use mamba in CIs (mne-tools#11471)
  Updating documentation to clarify full vs half-bandwidth and defaults in time_frequency.multitaper.py (mne-tools#11479)
  Fix typo in tutorial (mne-tools#11498)
  Typo fix and added colons before code (mne-tools#11496)
  [MRG] ENH/DOC: demo custom spectrum creation (mne-tools#11493)
  Accept only left-clicks for adding annotations (mne-tools#11491)
  [BUG, MRG] Fix pial surface loading, logging in locate_ieeg (mne-tools#11489)
  [ENH] Added unit_role to add non-breaking space between magnitude  and units (mne-tools#11469)
  MAINT: Fix CircleCI build (mne-tools#11488)
  [DOC] Updated decoding.SSD documentation and internal variable naming (mne-tools#11475)
  Typo fix (mne-tools#11485)
  [MRG] Forward argument axes from plot_sensors to DigMontage.plot (mne-tools#11470)
  [MRG] Improve error message raised on channels missing from DigMontage (mne-tools#11472)
  MAINT: Deal with pkg_resources usage bugs (mne-tools#11478)
  Add object array support and docstring (mne-tools#11465)
  [ENH] Adjusted SSD algorithm to support non-full rank data (mne-tools#11458)
  [BUG] fix nibabel reference (mne-tools#11467)
  ...
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.

mne.viz.plot_sensors accepts ch_groups in the type list (of lists) but this is undocumented

2 participants