-
Notifications
You must be signed in to change notification settings - Fork 708
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
gdf event reading #2413
gdf event reading #2413
Conversation
I think it's a clean PR, but I still need to dig up some example data and write a test function. Also, I should at some point clean up the empty merge messages that somehow keep lingering in my fork's history |
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_yorkinstruments, test_pull2050, test_read_trigger, test_issue1585, test_pull731, test_bug1262 When inside the DCCN, please also consider testing: test_bug1924, test_issue789, test_issue1972, test_pull769, test_bug1407, test_yorkinstruments, test_issue1585, test_bug2093, test_bug2060, test_pull1271, test_bug2887, test_bug1262, test_issue2026, test_bug3207, inspect_bug645, test_bug3027, test_pull731, test_bug629, test_bug2170, test_curry, test_bug2415, test_issue1387, test_pull1229, test_bug1266, test_bug1914, test_pull1248 Suggested tests outside the DCCN use public data or do not use data. |
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_pull731, test_issue1585, test_pull2050, test_bug1262, test_read_trigger, test_yorkinstruments When inside the DCCN, please also consider testing: test_pull1248, test_bug3027, test_bug1914, test_issue1387, test_bug1407, test_bug1262, test_bug3207, test_bug1924, test_bug2887, test_bug2170, inspect_bug645, test_pull1229, test_curry, test_pull1271, test_issue789, test_bug2415, test_bug1266, test_pull731, test_pull769, test_bug2093, test_issue1585, test_bug629, test_issue2026, test_bug2060, test_issue1972, test_yorkinstruments Suggested tests outside the DCCN use public data or do not use data. |
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_pull731, test_yorkinstruments, test_bug1262, test_issue1585, test_pull2050, test_read_trigger When inside the DCCN, please also consider testing: test_bug1407, test_bug629, test_bug1262, test_pull1229, test_issue2026, test_yorkinstruments, test_pull769, test_bug1266, test_pull1248, test_bug2060, test_bug3027, test_bug2170, inspect_bug645, test_curry, test_issue789, test_pull1271, test_bug1924, test_bug2415, test_issue1972, test_bug2887, test_bug1914, test_pull731, test_issue1387, test_bug3207, test_issue1585, test_bug2093 Suggested tests outside the DCCN use public data or do not use data. |
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_issue1585, test_read_trigger, test_pull731, test_pull2050, test_bug1262, test_yorkinstruments When inside the DCCN, please also consider testing: test_pull1229, test_bug1924, test_issue1387, test_pull769, test_pull1248, test_pull731, test_pull1271, test_bug2415, test_curry, test_issue789, test_bug629, inspect_bug645, test_bug2887, test_issue1972, test_issue1585, test_bug1266, test_bug2170, test_bug1407, test_bug2060, test_bug1262, test_yorkinstruments, test_bug1914, test_bug2093, test_bug3207, test_issue2026, test_bug3027 Suggested tests outside the DCCN use public data or do not use data. |
Aaargh, why does the diff now also include a change to data2bids? I will close this and create a new clean PR with only the intended changes. |
hmm, but upon second thought, the change to data2bids is due to a commit included by @robertoostenveld into this PR... Do you think it's safe to merge the PR from here? |
huh, what is my commit to data2bids doing here? Could you rebase your commit onto the master to clean it up? |
I'll try but always get nervous when I have to find out what voodoo steps and pinkel commands to execute |
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_yorkinstruments, test_pull731, test_read_trigger, test_issue1585, test_bug1262, test_pull2050 When inside the DCCN, please also consider testing: test_yorkinstruments, test_bug2093, test_bug629, test_issue1585, inspect_bug645, test_bug1266, test_pull1229, test_issue1387, test_bug2415, test_pull769, test_bug1262, test_bug2170, test_issue2026, test_bug1924, test_bug2060, test_bug3207, test_pull731, test_issue1972, test_pull1248, test_bug2887, test_issue789, test_bug3027, test_curry, test_bug1914, test_bug1407, test_pull1271 Suggested tests outside the DCCN use public data or do not use data. |
phew, the rebasing went more smoothly than I feared. Ready to be merged as far as I am concerned. |
looks good to me, I will merge. |
I think that my commit appeared on your gdf branch because of the following:
That caused my commits on master to appear on your gdf branch. By rebasing you moved your commits to a later point in time, after my commits. The diff between your rebased gdf and the (by now updated) master therefore did not show them any more. I think that without the rebase upon a merge, git would also have properly resolved the commits that in your branch were in a different order than on master. But better safe than sorry. |
Yup, indeed I think that all this started when I pulled the branch (which I started on my laptop) into the repo that I have on the cluster. I might have done a git pull upstream on the master branch before creating the gdf branch, and pulling it from origin. |
This is an attempt to read in events from a biosemi/gdf file, where the events are represented in the header (and not in the trigger channel)