Conversation
Automated Review URLs |
dstansby
left a comment
There was a problem hiding this comment.
Some comments, but 👍 this looks great overall
| See the [example below](spec:example:coordinate_transformation). | ||
|
|
||
| ```{hint} | ||
| [Multiscale images](#multiscale-md) generally have an "intrinsic" coordinate system, |
There was a problem hiding this comment.
Could "generally" be changed to either "are recommended to" or "always", to make it clearer whether they're always present, and if not recommended?
|
|
||
| ```{hint} | ||
| [Multiscale images](#multiscale-md) generally have an "intrinsic" coordinate system, | ||
| which will generally be a representation of the image in its native physical coordinate system. |
There was a problem hiding this comment.
What does "generally be a..." mean here? That it's an innacurate represenatation? That sometimes it isn't a represenation in a physical coordinate system?
As a general thought I think it's more confusing to have general statements like this in the spec that are not true for all datasets.
There was a problem hiding this comment.
The statement here was intended to introduce the term "intrinsic" up front, because it's used several times further down. I guess there are cases where the described image array (i.e., the "intrinsic" coordinate system) is different from the coordinate system in which one would want to natively view the image. For instance, in SCAPE microscopy, the acquired image stack is skewed, so the native stack is not something one would prefer to look at. But that's relatively niche and for 99% of all use cases, the statement probably holds.
| The values of `input` and `output` MUST be an object with the fields `name` and `path` that satisfy: | ||
| - The `path` field of `input` MUST be the same as the `path` field of the dataset, | ||
| the `name` field of `input` can be omitted. | ||
| Implementations should always treat the value of `path` under the `input` field as if it were equal to the value of `path`. |
There was a problem hiding this comment.
I don't understand the "path must be equal to path" constraint here? Possibly this is a hangover from the previous version and can just be deleted now?
There was a problem hiding this comment.
Yeah...it sounded clunky no matter how I put it.
Essentially, a dataset entry must contain
{
"path": "s0",
"coordinateTransformations": [
{
"type": "indentity",
"input": {"path": "s0"},
"output": {"name": "physical"}
}
]
}and I wanted to express that the contents of both path fields in here must be identical.
| The `path` field of `output` can be omitted. | ||
|
|
||
| The coordinate system referenced by all `output` fields of the coordinate transformations (the "intrinsic" coordinate system) | ||
| will generally be a representation of the image in its native physical coordinate system. |
There was a problem hiding this comment.
Again, what does "generally" mean?
index.md
Outdated
| which is different from the "intrinsic" coordinate system. | ||
| The values of both `input` and `output` fields MUST be an object with fields `name` and `path`. | ||
| The value of `input` MUST be the "intrinsic" coordinate system. | ||
| The value of `output` can be the name of an output coordinate System in the same multiscales group |
There was a problem hiding this comment.
| The value of `output` can be the name of an output coordinate System in the same multiscales group | |
| The value of `output` can be the name of an output coordinate system in the same multiscales group |
There was a problem hiding this comment.
Also, this currently implies value of output can be a string, is that correct?
There was a problem hiding this comment.
Ah, good point. I edited the section a bit in 9c7dae2
Co-authored-by: David Stansby <dstansby@gmail.com>
Co-authored-by: David Stansby <dstansby@gmail.com>
Co-authored-by: David Stansby <dstansby@gmail.com>
Co-authored-by: David Stansby <dstansby@gmail.com>
Co-authored-by: David Stansby <dstansby@gmail.com>
Fixes ome/ngff#480
Fixes ome/ngff#437
Relevant for ome/ngff#360
Description
In previous versions of the spec, we had used a mix of contentions when it came to specifying the
inputs andoutputs of coordinate transformations:scenecontext,inputandoutputhad to be an object with fieldsnameandpath.multiscales > datasets > coordinateTransformations), only a string was allowed, which had to correspond to the path of the respectivedatasetentry.multiscales > coordinateTransformations), a string was required, which had to correspond top the name of a coordinate system in the same metadata document.Following the original suggestion of @dstansby at ome/ngff#437, this is unified into a common syntax in this PR. It also has adjustments for schemas, examples and CI tests.
cc @will-moore @dstansby @clbarnes @bogovicj