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

Fix BTi handling in mne-bids #1160

Merged
merged 7 commits into from
Aug 5, 2023
Merged

Fix BTi handling in mne-bids #1160

merged 7 commits into from
Aug 5, 2023

Conversation

sappelhoff
Copy link
Member

@sappelhoff sappelhoff commented Aug 2, 2023

I think BTI copying was broken all this time in MNE-Python, I found several things that don't make any sense, and seemingly we never even had a test for this.

I now introduced a test. this code will only work with mne >= 1.5


@nugenta we need your help to test this fix.

Can you please:

  1. install the current development version of mne
  2. install the fix from this branch? see instructions for installing from a branch here: only copy BTI headshape file if present #1158 (comment)

and then run your pipeline?

Are all your issues fixed?

mne_bids/copyfiles.py Outdated Show resolved Hide resolved
@sappelhoff sappelhoff changed the title fix bti copyfile Fix BTi handling in mne-bids Aug 2, 2023
@sappelhoff sappelhoff added this to the 0.13 milestone Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #1160 (6e4f551) into main (0b06562) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 6e4f551 differs from pull request most recent head 3a9d522. Consider uploading reports for the commit 3a9d522 to get more accurate results

@@           Coverage Diff           @@
##             main    #1160   +/-   ##
=======================================
  Coverage   97.62%   97.63%           
=======================================
  Files          40       40           
  Lines        8625     8655   +30     
=======================================
+ Hits         8420     8450   +30     
  Misses        205      205           
Files Changed Coverage Δ
mne_bids/path.py 97.38% <ø> (ø)
mne_bids/tests/test_path.py 99.85% <ø> (ø)
mne_bids/copyfiles.py 97.76% <100.00%> (+0.05%) ⬆️
mne_bids/read.py 95.89% <100.00%> (+0.05%) ⬆️
mne_bids/tests/test_copyfiles.py 98.40% <100.00%> (+0.08%) ⬆️
mne_bids/tests/test_write.py 99.50% <100.00%> (+<0.01%) ⬆️
mne_bids/write.py 97.40% <100.00%> (ø)

mne_bids/tests/test_write.py Outdated Show resolved Hide resolved
mne_bids/tests/test_copyfiles.py Outdated Show resolved Hide resolved
@nugenta
Copy link

nugenta commented Aug 4, 2023

Hi - sorry for only getting to this now. Yes, installing the latest devel versions of both mne and mne-bids have solved the BTi copying problems. Thanks!

@sappelhoff
Copy link
Member Author

Hi - sorry for only getting to this now. Yes, installing the latest devel versions of both mne and mne-bids have solved the BTi copying problems. Thanks!

no worries, thanks @nugenta ... but did you install mne-bids from this branch? ... or just the mne-bids development version?

anyhow, I am going to merge this now. Please update your mne-bids development version and tell us if it still works -- thanks!

@sappelhoff sappelhoff merged commit abf4738 into mne-tools:main Aug 5, 2023
1 check passed
@sappelhoff sappelhoff deleted the bti2 branch August 5, 2023 18:52
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.

4 participants