-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
MRG, ENH: Remove 15-char limit for FIF #8574
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
Conversation
8018b4b to
b9e3560
Compare
|
@agramfort this is ready for review from my end. Once we're happy with this, I can make the corresponding changes in MNE-MATLAB. Should we try to get this into 0.22, or make it an early 0.23 PR? I'm leaning toward the latter so we at least get a few months in of dev use |
|
let's merge this after the release |
|
Agreed. @bloyl would you be interested in doing the companion PR to MNE-MATLAB? My MATLAB brain has atrophied over the last many years of using Python... |
b9e3560 to
7159bda
Compare
|
Companion PR is in mne-tools/mne-matlab#21, so these two are ready for review/merge from my end. |
agramfort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9872f05 to
014678c
Compare
|
it took me some time to understand that's why I suggested to rename to
ch_names_mapping
?
also is there a way to hide this for any public function?
|
e03f8bb to
84b7c2d
Compare
I'm not sure what you're referring to here -- I made this change...? Pushed a change to preserve the API of read_ctf_comp. Failures are just |
|
thanks @larsoner ! |
fiff-constantscommit, check'bads'handling/replacementmonkeypatchto make the tag empty on read, ensure it can still be written and readCloses #8348
Closes #8312