-
Notifications
You must be signed in to change notification settings - Fork 87
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
[ADD] describing fields in event.json
file
#1255
Conversation
…nd duration columns
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽♂️ |
for more information, see https://pre-commit.ci
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.
Thanks @JuliusWelzel I merged main into your branch, so please pull before continuing to work on this (main contains some CI fixes now).
As for your PR, I am happy with the proposed changes, but please observe my comments in the code.
Finally, please follow the steps in the second to last section of this document: https://github.com/mne-tools/mne-bids/blob/main/CONTRIBUTING.md#instructions-for-first-time-contributors
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1255 +/- ##
==========================================
- Coverage 97.61% 97.45% -0.17%
==========================================
Files 40 40
Lines 8685 8685
==========================================
- Hits 8478 8464 -14
- Misses 207 221 +14 ☔ View full report in Codecov by Sentry. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Hopefully all done, so should be good to go |
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.
one last thing to do, the rest looks great -- thanks
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
Thanks @JuliusWelzel! |
PR Description
As currently implemented, the events.json file being written does not contain information about the two required columns "onset" and "duration" in the .json file.
As those columns are REQURIED, I would propose adding some background information to the .json file every time events are written for people who are not familiar with the structure of the events.tsv files in.REQUIRED.
See #1254 for details.
event.json
file #1254Merge checklist
Maintainer, please confirm the following before merging.
If applicable: