Skip to content

Allow 2D coordinate or mask.#58

Merged
YooSunYoung merged 8 commits into
mainfrom
allow-2d-coordinate
Jun 22, 2026
Merged

Allow 2D coordinate or mask.#58
YooSunYoung merged 8 commits into
mainfrom
allow-2d-coordinate

Conversation

@YooSunYoung

@YooSunYoung YooSunYoung commented Jun 18, 2026

Copy link
Copy Markdown
Member

fixed #56
I updated the schema and fixed the IO helper according to the schema.
We didn't need to allow masks to be 2D but... just included the mask... masks are booleans so it shoudn't matter too much.

@YooSunYoung YooSunYoung requested a review from nvaytet June 18, 2026 14:40
"properties": {
"neutron_type": {"$ref": "#/$defs/NeutronSourceType"},
"wavelength_range": {"$ref": "#/$defs/ScippVariable"}
"wavelength_range": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I didn't quite get why this one is needed...

"title": "ScippVariable1D",
"type": "object"
},
"ScippVariable2D": {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure I followed: the 0D, 1D and 2D all seem to be identical?

In which place do we actually need the different types?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

At first I thought we would want to limit the 2D coordinate only for x, y dimension so I started splitting them into different Models so that 2D one can have literal type of dimensions but I wasn't sure if that's necessary so I removed it...

I guess we can just merge all three of them now. I'll update them.

@YooSunYoung

Copy link
Copy Markdown
Member Author

I ended up making each 0-2D variable model definition more strict, e.g. number of items in the dims and shape and the values structure should match.

Then the weird 2D object values naturally won't pass the validation and the error messages will tell you why.

@YooSunYoung YooSunYoung requested a review from nvaytet June 19, 2026 09:39
Comment on lines +110 to +114
"maxItems": 2,
"minItems": 2,
"prefixItems": [{ "$ref": "#/$defs/ScippVariable0D" }, { "$ref": "#/$defs/ScippVariable0D" }],
"title": "Wavelength Range",
"type": "array"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now the wavelength range has strict type of tuple[scalar, scalar]

"neutron_type": {"$ref": "#/$defs/NeutronSourceType"},
"wavelength_range": {"$ref": "#/$defs/ScippVariable"}
"wavelength_range": {
"anyOf": [{ "$ref": "#/$defs/ScippVariable0D" }, { "$ref": "#/$defs/ScippVariable1D" }, { "$ref": "#/$defs/ScippVariable2D" }],

@nvaytet nvaytet Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Change to a tuple containing exactly two 0D variables?

dims=('z',), shape=(1,), dtype='int', unit='m', values=[0]
),
'y': ScippVariable(
'y': ScippVariable1D(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't x and y coords be 2d?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is just a dummy metadata as an example in the documentation. I didn't include 2D coordinate since we don't want to encourage people to store 2D coordinates in the metadata.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry I had not seen it was creating some dummy data.

Comment thread src/scitiff/io.py Outdated
raise ValueError(
"Only 1-dimensional or scalar variable is allowed for metadata. "
"The variable has more than 1 dimension."
"Only 2, 1-dimensional or scalar variable is allowed for metadata. "

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"Only 2, 1-dimensional or scalar variable is allowed for metadata. "
"Only variables with at most 2 dimensions are allowed for metadata. "

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the error message...!

@YooSunYoung YooSunYoung merged commit 06a4db0 into main Jun 22, 2026
3 checks passed
@YooSunYoung YooSunYoung deleted the allow-2d-coordinate branch June 22, 2026 15:05
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.

Allow 2d coordinates along the x and y dimensions

2 participants