Repository navigation
Add v0.6 plate zarr groups/arrays as required - #35
DragaDoncila wants to merge 2 commits into
Conversation
kevinyamauchi
left a comment
kevinyamauchi
left a comment
There was a problem hiding this comment.
Thanks @DragaDoncila ! I think this looks good. I did a manual spot check and cross-validated with ome-zarr-models-py. I flagged a minor issue below where we have multiple types of errors in some invalid cases.
@clbarnes , what do you think? I think we can merge once we merge the suggestions in and you're happy with it. I'm traveling next week, so if you're happy with it, please feel free to merge.
| "well": { | ||
| "images": [ | ||
| { | ||
| "acquisition": 0, |
There was a problem hiding this comment.
| "acquisition": 0, | |
| "acquisition": -1, |
I think this should match the acquisition so the only issue is the negative ID. As is, this also invalid because the acquisition ID 0 is not defined.
| "well": { | ||
| "images": [ | ||
| { | ||
| "acquisition": 0, |
There was a problem hiding this comment.
| "acquisition": 0, | |
| "acquisition": "0", |
Similar to above, I think we should match this to the defined acquisition ID so the only error is that the acquisition id is a string.
2 participants
Part of #17
Add the missing row, well, multiscale groups and single level zarr array for each test that requires it. Tests where the intended defect IS a missing row/well/etc. are not touched.
Right now afaict all the defects are in the attributes file itself rather than in the on-disk layout e.g. we declare a row named A but we name the group B. I wasn't sure if adding tests such as these was in scope for this PR.