Skip to content

Commit

Permalink
Merge pull request #1 from yarikoptic/rfc-2-updates
Browse files Browse the repository at this point in the history
Add my re-review (ome#2) of RFC-2
  • Loading branch information
joshmoore authored Sep 12, 2024
2 parents 311e806 + 5c25ae7 commit 9f14407
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
6 changes: 6 additions & 0 deletions rfc/2/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ This RFC is currently in RFC state (R6).
- Google
- 2024-09-11
- [Accept](./reviews/3b/index)
* - Reviewer
- Yaroslav O. Halchenko
- yarikoptic
- Dartmouth College, DANDI Project
- 2024-09-11
- [Accept](./reviews/2b/index)
```

## Overview
Expand Down
25 changes: 25 additions & 0 deletions rfc/2/reviews/2b/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Review 2 (update)

## Review authors
This review was written by:
- Yaroslav O. Halchenko (Dartmouth College, DANDI Project)

## Summary

Comments from the initial round of reviews were proposed, accepted (with minor tune ups after) in https://github.com/ome/ngff/pull/250 . The changes to RFC-2 in that PR are sufficient to update our recommendation from "Major changes" to "Accept".

- Concern on compatibility with Zarr v2/OME 0.4 was largely addressed: it is now RECOMMENDED to maintain compatibility.
- Concern on v2 and v3 co-existence was addressed by more explicit formulation that it is possible but NOT RECOMMENDED [^1], and that 0.5 SHOULD be treated preferentially in case of both v2 and v3 metadata co-existence.
- Recommendation on separation of the "name" of the standard from the versioning was addressed by introduction of `ome` as the top-level key for the OME-NGFF metadata in zarr.json with a `version` key within that `ome` record.
- unfortunately the concept of "namespaces" is not yet formalized in Zarr itself, WiP in [ZEP004: Namespaces](https://github.com/zarr-developers/zarr-specs/pull/262/files).
So it would remain possible that changes in the future would be needed to align with Zarr's formalization of namespaces.
- My questioning of the value to bundle together support of Zarr v3 and changes to metadata was largely addressed by addition to "Drawbacks, risks, alternatives, and unknowns": even without formalization of "Zarr namespaces" it would be useful to have `ome` as the top-level key for the OME-NGFF metadata in zarr.json.

[^1]: according to [RFC 2119: Section 4](https://datatracker.ietf.org/doc/html/rfc2119#section-4), even though "NOT RECOMMENDED" is not explicitly listed among the list of key words, it is to be treated similarly to "SHOULD NOT", so we are fine here.

Remaining concerns in the original review are addressed by stronger language around how applications should handle multiple versions and the usage of ‘ome’ as the top-level key for the OME-NGFF metadata in zarr.json.


## Recommendation

"Accept".

0 comments on commit 9f14407

Please sign in to comment.