Skip to content

[DOC] extend documentation for add_channels#13051

Merged
hoechenberger merged 9 commits intomne-tools:mainfrom
skjerns:patch-1
Jan 14, 2025
Merged

[DOC] extend documentation for add_channels#13051
hoechenberger merged 9 commits intomne-tools:mainfrom
skjerns:patch-1

Conversation

@skjerns
Copy link
Copy Markdown
Contributor

@skjerns skjerns commented Jan 8, 2025

Fixes #11106.

What does this implement/fix?

I stumpled upon the documentation of add_channels and upon googleing saw that I was not the only one being confused on how to use the function.

Therefore, some additions that will make it easier to understand.

TODO: Some of the links are not referenced correctly, I don't know how to reference the auto examples. Can someone fix this? :)

@skjerns skjerns mentioned this pull request Jan 8, 2025
Closed
@skjerns
Copy link
Copy Markdown
Contributor Author

skjerns commented Jan 8, 2025

Sorry, don't understand the CI errors, can someone clarify?

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Jan 8, 2025

A bunch of failures should be fixed by #13049

The style one is weird as it is >= 3.10 on main already. Maybe merging main into your branch once #13049 is in will fix things

@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Jan 13, 2025 •
edited
Loading

It's difficult for me to review because the diff extends over dozens of files. I think it's because you opened the PR for merging into the maint/1.9 branch instead of main and merged changes from main into your branch

@skjerns skjerns changed the base branch from maint/1.9 to main January 13, 2025 11:20
@skjerns
Copy link
Copy Markdown
Contributor Author

skjerns commented Jan 13, 2025

It's difficult for me to review because the diff extends over dozens of files. I think it's because you opened the PR for merging into the maint/1.9 branch instead of main and merged changes from main into your branch

yep, indeed, that seemed to have happened. I'll changed to merge into main?

@hoechenberger
Copy link
Copy Markdown
Member

hoechenberger commented Jan 13, 2025

It's difficult for me to review because the diff extends over dozens of files. I think it's because you opened the PR for merging into the maint/1.9 branch instead of main and merged changes from main into your branch

yep, indeed, that seemed to have happened. I'll changed to merge into main?

That would be great! I'm not sure if that works with an existing PR, though, or if you'd have to close this one and open a ne one

cbrnr
cbrnr reviewed Jan 13, 2025
View reviewed changes
cbrnr
cbrnr reviewed Jan 13, 2025
View reviewed changes
cbrnr
cbrnr reviewed Jan 13, 2025
View reviewed changes
cbrnr
cbrnr reviewed Jan 13, 2025
View reviewed changes
@skjerns
Copy link
Copy Markdown
Contributor Author

skjerns commented Jan 13, 2025

I've reverted the changes, not sure how they ended up in the commit, probably because of merging 1.9 into it dev beofre.

It was a bit messy though, let me know if you want me to open a new PR to have a cleaner commit history.

@cbrnr
Copy link
Copy Markdown
Contributor

cbrnr commented Jan 13, 2025 •
edited
Loading

I've reverted the changes, not sure how they ended up in the commit, probably because of merging 1.9 into it dev beofre.

Yep!

It was a bit messy though, let me know if you want me to open a new PR to have a cleaner commit history.

No, looking good right now! We squash and merge anyway.

hoechenberger
hoechenberger reviewed Jan 13, 2025
View reviewed changes
hoechenberger
hoechenberger reviewed Jan 13, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

looks great, just a minor suggestion

@cbrnr
Copy link
Copy Markdown
Contributor

cbrnr commented Jan 13, 2025

Do you want to add a changelog entry? If you think this is not necessary let me know and I'll add the corresponding label here so that the test passes.

@skjerns @hoechenberger
channels.py aktualisieren
4832d6a 
Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@skjerns
Copy link
Copy Markdown
Contributor Author

skjerns commented Jan 13, 2025

Do you want to add a changelog entry? If you think this is not necessary let me know and I'll add the corresponding label here so that the test passes.

Not necessary!

@drammock
Copy link
Copy Markdown
Member

drammock commented Jan 13, 2025 

mne/channels/channels.py:697:89: E501 Line too long (97 > 88)

/home/circleci/project/mne/epochs.py:docstring of mne.channels.channels.UpdateChannelsMixin.add_channels:46: WARNING: py:class reference target not found: mne.Raw [ref.class]

(should be mne.io.Raw not mne.Raw)

wmvanvliet
wmvanvliet reviewed Jan 14, 2025
View reviewed changes
sappelhoff
sappelhoff approved these changes Jan 14, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

Good work, this clarifies how to use the function by a lot!

@hoechenberger hoechenberger enabled auto-merge (squash) January 14, 2025 10:36
@hoechenberger hoechenberger merged commit 5fec4e0 into mne-tools:main Jan 14, 2025
@cbrnr
Copy link
Copy Markdown
Contributor

cbrnr commented Jan 14, 2025

Thanks @skjerns!

larsoner added a commit that referenced this pull request Jan 14, 2025
@larsoner
Merge remote-tracking branch 'upstream/main' into pre-commit-ci-updat…
7881c4c 
…e-config

* upstream/main:
  [DOC] extend documentation for add_channels (#13051)
  Add `combine_tfr` to API (#13054)
  Add `combine_spectrum()` function and allow `grand_average()` to support `Spectrum` data (#13058)
  BUG: Fix bug with helium anon (#13056)
larsoner added a commit to emma-bailey/mne-python that referenced this pull request Jan 14, 2025
@larsoner
Merge remote-tracking branch 'upstream/main' into ebm
72facf0 
* upstream/main:
  [DOC] extend documentation for add_channels (mne-tools#13051)
  Add `combine_tfr` to API (mne-tools#13054)
  Add `combine_spectrum()` function and allow `grand_average()` to support `Spectrum` data (mne-tools#13058)
  BUG: Fix bug with helium anon (mne-tools#13056)
  [ENH] Add option to store and return TFR taper weights (mne-tools#12910)
qian-chu pushed a commit to qian-chu/mne-python that referenced this pull request Jan 20, 2025
@skjerns @qian-chu
[DOC] extend documentation for add_channels (mne-tools#13051)
56a1e42
larsoner added a commit to larsoner/mne-python that referenced this pull request Jan 24, 2025
@larsoner
Merge remote-tracking branch 'upstream/main' into cola5
46c193d 
* upstream/main: (57 commits)
  Allow lasso selection sensors in a plot_evoked_topo (mne-tools#12071)
  MAINT: Fix doc build (mne-tools#13076)
  BUG: Improve sklearn compliance (mne-tools#13065)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13073)
  MAINT: Add Numba to 3.13 test (mne-tools#13075)
  Bump autofix-ci/action from ff86a557419858bb967097bfc916833f5647fa8c to 551dded8c6cc8a1054039c8bc0b8b48c51dfc6ef in the actions group (mne-tools#13071)
  [BUG] Correct annotation onset for exportation to EDF and EEGLAB (mne-tools#12656)
  New feature for removing heart artifacts from EEG or ESG data using a Principal Component Analysis - Optimal Basis Sets (PCA-OBS) algorithm (mne-tools#13037)
  [BUG] Fix taper weighting in computation of TFR multitaper power (mne-tools#13067)
  [FIX] Reading an EDF with preload=False and mixed frequency (mne-tools#13069)
  Fix evoked topomap colorbars, closes mne-tools#13050 (mne-tools#13063)
  [pre-commit.ci] pre-commit autoupdate (mne-tools#13060)
  BUG: Fix bug with interval calculation (mne-tools#13062)
  [DOC] extend documentation for add_channels (mne-tools#13051)
  Add `combine_tfr` to API (mne-tools#13054)
  Add `combine_spectrum()` function and allow `grand_average()` to support `Spectrum` data (mne-tools#13058)
  BUG: Fix bug with helium anon (mne-tools#13056)
  [ENH] Add option to store and return TFR taper weights (mne-tools#12910)
  BUG: viz plot window's 'title' argument showed no effect. (mne-tools#12828)
  MAINT: Ensure limited set of tests are skipped (mne-tools#13053)
  ...
@skjerns skjerns deleted the patch-1 branch January 27, 2026 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wmvanvliet wmvanvliet wmvanvliet approved these changes

@cbrnr cbrnr cbrnr left review comments

@sappelhoff sappelhoff sappelhoff approved these changes

@agramfort agramfort Awaiting requested review from agramfort agramfort is a code owner

@mscheltienne mscheltienne Awaiting requested review from mscheltienne mscheltienne is a code owner

@dengemann dengemann Awaiting requested review from dengemann

@jasmainak jasmainak Awaiting requested review from jasmainak

@larsoner larsoner Awaiting requested review from larsoner

@drammock drammock Awaiting requested review from drammock

@adam2392 adam2392 Awaiting requested review from adam2392

@alexrockhill alexrockhill Awaiting requested review from alexrockhill

+1 more reviewer

@hoechenberger hoechenberger hoechenberger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

no-changelog-entry-needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add_channels() documentation is difficult to understand

7 participants

@skjerns @larsoner @hoechenberger @cbrnr @drammock @wmvanvliet @sappelhoff