Skip to content

Port RFC5 updated proposal to ngff-spec#67

Open
jo-mueller wants to merge 46 commits intoome:mainfrom
jo-mueller:update-RFC5
Open

Port RFC5 updated proposal to ngff-spec#67
jo-mueller wants to merge 46 commits intoome:mainfrom
jo-mueller:update-RFC5

Conversation

@jo-mueller
Copy link
Contributor

@jo-mueller jo-mueller commented Jan 15, 2026

This is the PR following up to ome/ngff#389.

This PR

  • inserts the proposed text into the full spec document
  • makes sure that the correct formatting and cross-referencing is used
  • adds some more examples to the transformations
  • update and add new schemas
  • adds a summary of changes to the changelog
  • add scene example metadata

PRs to merge first:

@github-actions
Copy link

Automated Review URLs

@jo-mueller
Copy link
Contributor Author

@dstansby updated the schemas! Could I ask you to have a swift look if something strikes you as off?

@dstansby
Copy link
Contributor

I had a look, and I'm struggling to read the schemas by eye, and the rendering doesn't look so nice either (https://ngff-spec--67.org.readthedocs.build/en/67/schemas/scene/, https://ngff-spec--67.org.readthedocs.build/en/67/schemas/coordinate-transformations/, https://ngff-spec--67.org.readthedocs.build/en/67/schemas/coordinate-systems/)

Perhaps a better test than manual review is checking that the examples all validate correctly using the schemas?

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Jan 19, 2026

and I'm struggling to read the schemas by eye

I feel you 🙄

and the rendering doesn't look so nice either

Tbf, it doesn't look very different from the rendering on the current main. I think it's getting especially nasty for the OneOf of AllOf clauses in the schema, but I'm afraid the rendering is what it is. We were thinking about experimenting with LinkML for the schemas (maybe these render nicer?) but that's a different discussion.

But you're right about the validator - I guess the easiest thing is then to create fork of the validator, point it to this branch for the schemas and check the examples 👍 OR simply copy some of the examples to the testing CI in here.

Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had more of a look at the RTD build, and some initial comments:

There's "Coordinate systems" and "Coordinate systems and transforms" pages, which seems confusing (duplication of coordinate systems):
Screenshot 2026-01-19 at 10 11 36

I don't understand why coordinate systems and transformations are in the same schema, wouldn't it be much cleaner to have them in separate schemas?

On a similar theme, it would be much nicer to have all the different transforms as their own schema that are then referenced by other schemas. It would incraese the number of schema files, but make it much easier to find a given transformation for example.

@jo-mueller
Copy link
Contributor Author

There's "Coordinate systems" and "Coordinate systems and transforms" pages, which seems confusing (duplication of coordinate systems):

I think that's a legacy thing. They maybe used to be but the schemas are separated as you suggest. It was just that the title of the coordinate transformations schema file was still "coordinate systems and transformations" - I changed it, should be fixed now.

On a similar theme, it would be much nicer to have all the different transforms as their own schema that are then referenced by other schemas. It would incraese the number of schema files, but make it much easier to find a given transformation for example.

Agree! Makes total sense and can do.

@dstansby
Copy link
Contributor

Yep, no problem 👍

@jo-mueller jo-mueller marked this pull request as ready for review February 4, 2026 11:22
@jo-mueller jo-mueller changed the title WIP: port RFC5 updated proposal to ngff-spec Port RFC5 updated proposal to ngff-spec Feb 4, 2026
@jo-mueller
Copy link
Contributor Author

@dstansby I added a bunch of the examples from jo-mueller/ngff-rfc5-coordinate-transformation-examples to the test cases here. They all check out alright, except for one byDimension test case, for which I can't seem to be able to figure out what is wrong with it (tests > attributes > spec > valid > transforms > byDimension.json). Does something catch your eye, by any chance? 😕

whose values MUST be an array of valid [coordinate systems](#coordinate-systems-md).

If used inside "scene" metadata, the `input` and `output` fields of `coordinateTransformations` MUST contain a json object,
which MUST contain either the `path` or the `name` field, or both.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is always required for input/output. Can't have just path.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If NOT used inside a "scene" the coordinateTransform input and output fields can be strings OR objects.
If NOT used inside a "scene" the schema also allows input and output to be empty objects (since neither "name" nor "path" are required). Is this correct or does the schema need fixing?
These details aren't mentioned here.

Copy link
Contributor Author

@jo-mueller jo-mueller Feb 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@will-moore good catch, the name must always be there. I am a bit hesitant to make changes to the text here because I was planning for this PR to only mirror the latest RFC5 proposal. I'll create an issue on ngff to make sure we don't forget later?

@jo-mueller
Copy link
Contributor Author

@dstansby found it! The failing example (byDImension.json) used an array coordinate system, which hsa two axes of "type": "array". The coordinate system schema requires at least two spatial axes though. I need to check whether that's actually enforced by the spec.

Otherwise, axes MUST have two or three spatial axes, which is forbidden for multiscales coordinate systems
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants