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

[ENH] MNEScan FTBuffer plugin - Channel names compatibility #840

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

juangpc
Copy link
Collaborator

@juangpc juangpc commented Sep 22, 2021

No description provided.

@juangpc
Copy link
Collaborator Author

juangpc commented Sep 22, 2021

Ready for review. 😟

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2021

Codecov Report

Merging #840 (5d6a36f) into main (dbd57f7) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #840      +/-   ##
==========================================
- Coverage   21.80%   21.78%   -0.03%     
==========================================
  Files         451      451              
  Lines       39087    39077      -10     
==========================================
- Hits         8522     8512      -10     
  Misses      30565    30565              
Impacted Files Coverage Δ
libraries/fiff/c/fiff_coord_trans_old.cpp 25.74% <ø> (ø)
libraries/fiff/fiff_id.cpp 80.00% <ø> (-2.82%) ⬇️
libraries/fiff/fiff_ch_info.cpp 100.00% <100.00%> (ø)

@juangpc juangpc changed the title [ENH] FT plugin Channel names definitions are no compatible with FTPlugin [ENH] MNEScan FTBuffer plugin - Channel names definitions compatibility Sep 23, 2021
@juangpc juangpc changed the title [ENH] MNEScan FTBuffer plugin - Channel names definitions compatibility [ENH] MNEScan FTBuffer plugin - Channel names compatibility Sep 23, 2021
@gabrielbmotta
Copy link
Collaborator

I'm not sure what happened but there are a bunch of old commits added again in this pr

@juangpc
Copy link
Collaborator Author

juangpc commented Sep 29, 2021

Should be fixed now.

@juangpc
Copy link
Collaborator Author

juangpc commented Nov 3, 2021

@LorenzE @gabrielbmotta Please review this when you can.

data.info.meas_date[1] = 0;
}

if ( data.info.sfreq <= 0 )
Copy link
Member

Choose a reason for hiding this comment

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

as long as we have not set on a specific formatting rule, I would propose follow the old style aka no spaces before and after (as well as )

// fiff_int_t machid[2]; /**< Unique machine ID. */
// fiffTimeRec time; /**< Time of the ID creation. */
//} fiffIdRec,*fiffId; /**< This is the file identifier. */
//typedef fiffIdRec fiff_id_t;
Copy link
Member

Choose a reason for hiding this comment

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

I always found having these MNE C structs very useful from time to time. Any reason why you want to delete them?

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