Skip to content
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.

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

Exclude GDF channels #8520

Merged
merged 6 commits into from Nov 14, 2020
Merged

Exclude GDF channels #8520

merged 6 commits into from Nov 14, 2020

Conversation

cbrnr
Copy link
Contributor

@cbrnr cbrnr commented Nov 13, 2020

Fixes #8511. It seems like we neither implemented nor tested this feature. Let's see if existing tests pass, I'm going to add a specific test next.

@agramfort
Copy link
Member

@cbrnr feel free to merge and backport

thanks a lot

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 14, 2020

Is there a short description on how to backport specific PRs? I don't remember how I did it because that's been a long time.

@larsoner
Copy link
Member

Nowadays I:

  1. Squash and merge
  2. Locally check out maint/0.21 up to date and tracking upstream branch
  3. Cherry pick the squashed commit
  4. Git mergetool to deal with the deleted file and any conflicts (usually just means hit "d" to delete latest.inc that doesn't exist on maint)
  5. Manually copy-paste the change log line to 0.21.inc
  6. Git add 0.21.inc
  7. Git cherry pick --continue
  8. Accept the log message
  9. Git push

Sorry for the bad formatting, on mobile. But maybe this is enough hints. If not I can flesh out out later

@cbrnr cbrnr merged commit 5efddb9 into mne-tools:master Nov 14, 2020
4 of 5 checks passed
@cbrnr cbrnr deleted the gdf-exclude-chs branch November 14, 2020 16:54
cbrnr added a commit that referenced this pull request Nov 14, 2020
* Exclude GDF channels

* Fix units

* Add test

* Add changelog entry

* Fix PEP8

* Fix GDF2 units
@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 14, 2020

Thanks @larsoner, this worked perfectly! Should we add these instructions to our dev docs?

One more question, I've pushed the changes directly to upstream/maint/0.21 - was this correct or should I have used the branch on my fork? In any case, now I still see the "Create PR" message on the upstream MNE repository, how can I get rid of that?

@larsoner
Copy link
Member

Yes push to upstream is correct.

No way to get rid of that prompt that I know of, but lately it has gone away on its own, I think even the same day sometimes

@larsoner
Copy link
Member

Feel free to add this information to the wiki, it's probably better there than the dev docs. Only maintainers really need to care about this, similar to our release instructions (which also live in the wiki)

@drammock
Copy link
Member

There's been talk of an advanced dev docs page getting added but I think I agree with @larsoner that backporting is rare enough that the wiki is the best place for it.

@cbrnr
Copy link
Contributor Author

cbrnr commented Nov 14, 2020

Thanks, I've added a page to the wiki: https://github.com/mne-tools/mne-python/wiki/How-to-backport-a-change

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.

Excluding channels fails in GDF files
4 participants