Conversation
|
Hello @legouee! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2022-07-01 13:20:08 UTC |
JuliaSprenger
left a comment
JuliaSprenger
left a comment
There was a problem hiding this comment.
It seems like the mapping between the neo nwb structure is still developing. Maybe it would be good to introduce a mapping version that facilitates the correct reading of the nwb file?
neo/io/nwbio.py
Outdated
| nwbfile.add_epoch_column('_name', 'the name attribute of the Epoch') | ||
| # nwbfile.add_epoch_column('_description', 'the description attribute of the Epoch') | ||
| nwbfile.add_epoch_column( | ||
| 'segment', 'the name of the Neo Segment to which the Epoch belongs') | ||
| nwbfile.add_epoch_column('block', | ||
| 'the name of the Neo Block to which the Epoch belongs') | ||
| nwbfile.add_trial_column('segment', 'name of the Segment to which the Epoch belongs') | ||
| nwbfile.add_trial_column('block', 'name of the Block to which the Epoch belongs') |
There was a problem hiding this comment.
if nwb is supporting the epoch concept, what is the reason to switch to trials now?
There was a problem hiding this comment.
Yes. You are right. As it supports the epoch concept, it is not necessary to have the trials function.
I put it back as before.
| return tS_evt | ||
|
|
||
| def _write_epoch(self, nwbfile, epoch): | ||
| def _write_manage_epoch(self, nwbfile, segment, epoch, arr): |
There was a problem hiding this comment.
This method could benefit from having a docstring. What exactly is arr here?
There was a problem hiding this comment.
arr is an epoch array with t_start and t_stop sorted in ascending order.
I don't think it's necessary to create a docstring for this function. It's just an intermediate step in order to respect NWBInspector.
There was a problem hiding this comment.
If you sort t_start and t_stop independently, doesn't this corrupt data in case one epoch duration is contained in another?
| ): | ||
| for j in [label]: | ||
| t_stop = t_start + duration | ||
| seg_name = "%s %s" % (epoch.segment.name, label) |
There was a problem hiding this comment.
Here occurs the same error as with the events earlier. You have to keep a reference to the segment from before the proxy object is loaded.
4 participants
NWB Inspector is a new tool to help inspect NWB files for compliance with NWB Best Practices :
https://github.com/NeurodataWithoutBorders/nwbinspector
The following pull request allows the validation of files created with Neo with respect to NWB Inspector.