Skip to content

schema: fix reference to axes to use URLs, too - #203

Merged
jo-mueller merged 2 commits into
ome:mainfrom
jo-mueller:patch-fix-axes-reference
Sep 9, 2026
Merged

schema: fix reference to axes to use URLs, too#203
jo-mueller merged 2 commits into
ome:mainfrom
jo-mueller:patch-fix-axes-reference

Conversation

@jo-mueller

@jo-mueller jo-mueller commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Pointed out by @will-moore more here. The reference to the axes schema doesn't use the correct format because axes are not defined in the same schema document. This likely will need to be fixed for 0.1 .... 0.5 as well.

Edit: The axes schema doesn't sit in its own document for version 0.1...0.5, so this doesn't need to be backported 👍

@jo-mueller jo-mueller added the bug Something isn't working label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Automated Review URLs

Comment thread schemas/coordinate_systems.schema Outdated
@jo-mueller

jo-mueller commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

I was also just looking at the 0.5 branch, where axes don't exist in their own separate file, so it doesn't need to be referenced via an URL. I think this would be the only place where this fix needs to be made.

In fact, I don't see a compelling reason why the axes.schema has to be its own file (that a schema resolver would need to find) in the first place. axes metadata is only ever used inside coordinateSystems metadata. This design choice may have historic reasons (@bogovicj ?).

If nothing speaks against it, I could inline the axes.schema into the coordinate_systems.schema in a follow-up PR. Unless I end up breaking the validator or something like that

@will-moore

Copy link
Copy Markdown
Member

@jo-mueller Currently, the validator will try to load a axes.schema for v0.6 since it's hard-coded.
But with https://github.com/ome/ome-ngff-validator/pull/86/changes we detect find which schemas need loading dynamically. So it would be nice to get that in (which depends on ome/ngff#581) before inlining the axes.schema.

@lubianat lubianat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't have the complete background, but it is self contained and I trust @jo-mueller with the decision.

@jo-mueller
jo-mueller merged commit cf12979 into ome:main Sep 9, 2026
4 checks passed
@jo-mueller
jo-mueller deleted the patch-fix-axes-reference branch September 9, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants