Improved support for saving/loading Groups in NeoMatlabIO #1386
Conversation
This is done by saving references to objects inside groups, on the assumption that those objects are already stored somewhere within segments. This assumption may not always be true, so probably we should only save references for objects that would otherwise be duplicated.
Implement support for saving/loading Groups in NeoMatlabIO.
There was a problem hiding this comment.
Just some initial comments @apdavison, I'll test this when I get to lab in a couple hours and reread again.
| "your scipy version is too old to support " | ||
| + "MatlabIO, you need at least 0.12.0. " | ||
| + "You have %s" % scipy.version.version |
There was a problem hiding this comment.
| "your scipy version is too old to support " | |
| + "MatlabIO, you need at least 0.12.0. " | |
| + "You have %s" % scipy.version.version | |
| f"your scipy version is too old to support " | |
| + f"MatlabIO, you need at least 0.12.0. " | |
| + f"You have {scipy.version.version}" |
I'm drafting a PR for after the release to convert everything to f-strings.
|
|
||
|
|
||
| def get_classname_from_container_name(container_name, struct): | ||
| if container_name == "regionsofinterest": |
There was a problem hiding this comment.
Is there a reason why the container_name is regions, but all of the sub types are just region (no s). Just so I can know in the future?
There was a problem hiding this comment.
The container can contain multiple items, so it has a plural name, each item is a singular region.
| epoch_duration_range=[0.5, 3.0], | ||
| # this should be multiplied by pq.s, no? |
There was a problem hiding this comment.
| epoch_duration_range=[0.5, 3.0], | |
| # this should be multiplied by pq.s, no? | |
| epoch_duration_range=[0.5, 3.0], # this is multiplied by pq.s when generating the epoch below. |
I think we can edit this because the durations are loaded up and mulitplied by pq.s in the epoch generation below. Just so people don't get confused.
There was a problem hiding this comment.
not really relevant to this PR, we should perhaps have a project-wide comment cleanup
There was a problem hiding this comment.
True. I think the Black hitting things makes things tricky to review since it highlights all the lines. But you're right. We can do this later. It's not pressing.
|
Thanks @apdavison , I just want to do a test on my end where I write a file and I open it in matlab to see what it looks like. Then I'll approve. |
|
I have checked writing |
Replaces #1360. NeoMatlabIO now supports all Neo object types, including ImageSequence, ChannelView and RegionOfInterest.