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

[MRG] Fixes electrodes.tsv and coordsystem.json writing for EEG/iEEG to adhere to specification #604

Merged
merged 4 commits into from
Oct 26, 2020

Conversation

adam2392
Copy link
Member

@adam2392 adam2392 commented Oct 24, 2020

PR Description

Closes: #592

  • electrodes and coordsystem files for EEG and iEEG data should only have subject/session/acquisition/space/suffix defined.
  • simplified the arguments for _write_dig_bids using BIDSPath.

Merge checklist

Maintainer, please confirm the following before merging:

  • All comments resolved
  • This is not your own PR
  • All CIs are happy
  • PR title starts with [MRG]
  • whats_new.rst is updated
  • PR description includes phrase "closes <#issue-number>"

Copy link
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.

LGTM except for CI failures and a minor comment, thanks Adam!

doc/whats_new.rst Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Oct 25, 2020

Codecov Report

Merging #604 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #604   +/-   ##
=======================================
  Coverage   92.74%   92.75%           
=======================================
  Files          17       17           
  Lines        2302     2305    +3     
=======================================
+ Hits         2135     2138    +3     
  Misses        167      167           
Impacted Files Coverage Δ
mne_bids/dig.py 90.65% <100.00%> (+0.20%) ⬆️
mne_bids/write.py 95.51% <100.00%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a035c7...24eb10d. Read the comment docs.

@hoechenberger hoechenberger merged commit e7b7ea7 into mne-tools:master Oct 26, 2020
@hoechenberger
Copy link
Member

Thanks, @adam2392!

@adam2392 adam2392 deleted the elecs branch March 6, 2021 20:38
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.

minor bug?: electrodes.tsv and coordsystem.json writing per eeg/ieeg should only have certain entities
4 participants