-
Notifications
You must be signed in to change notification settings - Fork 90
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
MRG: Write events.json with "sample", "value", and "trial_type" entries #1132
MRG: Write events.json with "sample", "value", and "trial_type" entries #1132
Conversation
177828e
to
da449e4
Compare
fname = Path(fname) | ||
if fname.exists(): | ||
orig_data = json.loads( | ||
fname.read_text(encoding='utf-8'), | ||
object_pairs_hook=OrderedDict | ||
) | ||
new_data = {**orig_data, **new_data} | ||
|
||
_write_json(fname, new_data, overwrite) |
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.
Code duplication from below. But I felt it wasn't worth creating a new helper function for this. thoughts?
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.
looks fine to me
bb50fd2
to
6c2e9ed
Compare
1f6e157
to
e4a0c82
Compare
e4a0c82
to
5f2bc95
Compare
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 for this PR!
""" | ||
new_data = { | ||
'sample': { | ||
'Description': 'The event onset time in number of sampling points.' |
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.
needs clarification whether 0based or 1based indexing is used
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.
I don't know which one we use, and I don't know how it interacts with raw.first_samp
Could you help?
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.
I don't know either, but let's merge this and make a new issue to improve on this later. raw.first_samp
is always confusing :)
Codecov Report
@@ Coverage Diff @@
## main #1132 +/- ##
=======================================
Coverage 95.27% 95.28%
=======================================
Files 40 40
Lines 8682 8694 +12
=======================================
+ Hits 8272 8284 +12
Misses 410 410
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Fixes #1130
Also adding
trial_type
here because it's also considered an optional column that should come with an accompanying entry inevents.json
Not adding
Levels
fortrial_type
here, though – this would need discussion