Skip to content

4789 adds a space type metadata field to metatensor#4790

Merged
wyli merged 2 commits intoProject-MONAI:devfrom
wyli:4789-coordinates
Jul 29, 2022
Merged

4789 adds a space type metadata field to metatensor#4790
wyli merged 2 commits intoProject-MONAI:devfrom
wyli:4789-coordinates

Conversation

@wyli
Copy link
Copy Markdown
Contributor

@wyli wyli commented Jul 28, 2022

Signed-off-by: Wenqi Li [email protected]

Fixes #4789

Description

  • adds MetaTensor.meta['space'] defaulting to RAS
  • reviewed the readers and writers to support this key
  • added MetaKeys and SpaceKeys Enum

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@wyli
adds meta[space]
2e018a2 
Signed-off-by: Wenqi Li <[email protected]>
@wyli
fixes unit test
38c873e 
Signed-off-by: Wenqi Li <[email protected]>
@wyli wyli marked this pull request as ready for review July 28, 2022 16:25
Nic-Ma
Nic-Ma approved these changes Jul 29, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

The overall logic looks good to me.
But I think maybe we should call it orientation instead of space?
According to the ITK naming:
image

CC @ericspod @KumoLiu

Thanks.

@wyli
Copy link
Copy Markdown
Contributor Author

wyli commented Jul 29, 2022 •
edited
Loading

/build
Hi @Nic-Ma that itk orientation is relative to the base coordinate type (LPS), computed from the affine. The PR is about tracking the underlying base coordinate type. they are different concepts. I can see it's easily confusing but you can think of the difference between setting the reader flag vs using the orientation transform.

@wyli wyli enabled auto-merge (squash) July 29, 2022 06:08
@wyli wyli merged commit c1eaf42 into Project-MONAI:dev Jul 29, 2022
@wyli wyli deleted the 4789-coordinates branch September 7, 2022 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@KumoLiu KumoLiu KumoLiu approved these changes

@ericspod ericspod Awaiting requested review from ericspod ericspod is a code owner

@rijobro rijobro Awaiting requested review from rijobro

@yiheng-wang-nv yiheng-wang-nv Awaiting requested review from yiheng-wang-nv

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

metatensor to track the coordinate system

3 participants

@wyli @Nic-Ma @KumoLiu