-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve Orientation transform to use the "space" (LPS vs RAS) of a metatensor by default #8473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
e47dd21
a420cbd
b7fb59c
12f7dcc
1d389a2
28fc3af
78a35d1
620db36
6905382
6e5236d
ed7d5af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,7 @@ | |
ensure_tuple_rep, | ||
fall_back_tuple, | ||
) | ||
from monai.utils.deprecate_utils import deprecated_arg_default | ||
from monai.utils.enums import TraceKeys | ||
from monai.utils.module import optional_import | ||
|
||
|
@@ -545,12 +546,21 @@ class Orientationd(MapTransform, InvertibleTransform, LazyTransform): | |
|
||
backend = Orientation.backend | ||
|
||
@deprecated_arg_default( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should also have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried this already actually but then monai becomes unimportable, so I'm not really sure how this is supposed to work 🤷: >>> import monai
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/cpb28/Developer/monai/monai/__init__.py", line 101, in <module>
load_submodules(sys.modules[__name__], False, exclude_pattern=excludes)
File "/Users/cpb28/Developer/monai/monai/utils/module.py", line 187, in load_submodules
mod = import_module(name)
^^^^^^^^^^^^^^^^^^^
File "/Users/cpb28/.pyenv/versions/3.12.1/lib/python3.12/importlib/__init__.py", line 90, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/cpb28/Developer/monai/monai/apps/__init__.py", line 14, in <module>
from .datasets import CrossValidation, DecathlonDataset, MedNISTDataset, TciaDataset
File "/Users/cpb28/Developer/monai/monai/apps/datasets.py", line 33, in <module>
from monai.data import (
File "/Users/cpb28/Developer/monai/monai/data/__init__.py", line 29, in <module>
from .dataset import (
File "/Users/cpb28/Developer/monai/monai/data/dataset.py", line 39, in <module>
from monai.transforms import Compose, Randomizable, RandomizableTrait, Transform, convert_to_contiguous, reset_ops_id
File "/Users/cpb28/Developer/monai/monai/transforms/__init__.py", line 241, in <module>
from .io.array import SUPPORTED_READERS, LoadImage, SaveImage, WriteFileMapping
File "/Users/cpb28/Developer/monai/monai/transforms/io/array.py", line 31, in <module>
from monai.data import image_writer
File "/Users/cpb28/Developer/monai/monai/data/image_writer.py", line 23, in <module>
from monai.transforms.spatial.array import Resize, SpatialResample
File "/Users/cpb28/Developer/monai/monai/transforms/spatial/array.py", line 551, in <module>
class Orientation(InvertibleTransform, LazyTransform):
File "/Users/cpb28/Developer/monai/monai/transforms/spatial/array.py", line 561, in Orientation
@deprecated_arg_default(
^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/cpb28/Developer/monai/monai/utils/deprecate_utils.py", line 313, in _decorator
raise ValueError(
ValueError: Argument `labels` was replaced to the new default value `None` before the specified version 1.6. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK let's leave that off and it'll be something we revisit when looking at the deprecated items before the next release. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK so is there anything left to do on this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please also specify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @KumoLiu, I'm happy to make whatever changes necessary but I just don't really understand how this is supposed to work. I feel like I'm missing something here. If I specify
Similarly, if I specify
If I do both at the same time (as in your comment), I still get:
In any of these cases, all tests fail of course. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @KumoLiu I think we proceed with this one now and see about adjusting the arguments later along with other things that may need removing for 1.6. If you're ok we can resolve the comment and merge. Thank! |
||
name="labels", | ||
old_default=(("L", "R"), ("P", "A"), ("I", "S")), | ||
new_default=None, | ||
msg_suffix=( | ||
"Default value changed to None meaning that the transform now uses the 'space' of a " | ||
"meta-tensor, if applicable, to determine appropriate axis labels." | ||
), | ||
) | ||
def __init__( | ||
self, | ||
keys: KeysCollection, | ||
axcodes: str | None = None, | ||
as_closest_canonical: bool = False, | ||
labels: Sequence[tuple[str, str]] | None = (("L", "R"), ("P", "A"), ("I", "S")), | ||
labels: Sequence[tuple[str, str]] | None = None, | ||
allow_missing_keys: bool = False, | ||
lazy: bool = False, | ||
) -> None: | ||
|
@@ -564,7 +574,14 @@ def __init__( | |
as_closest_canonical: if True, load the image as closest to canonical axis format. | ||
labels: optional, None or sequence of (2,) sequences | ||
(2,) sequences are labels for (beginning, end) of output axis. | ||
Defaults to ``(('L', 'R'), ('P', 'A'), ('I', 'S'))``. | ||
If ``None``, an appropriate value is chosen depending on the | ||
value of the ``"space"`` metadata item of a metatensor: if | ||
``"space"`` is ``"LPS"``, the value used is ``(('R', 'L'), | ||
('A', 'P'), ('I', 'S'))``, if ``"space"`` is ``"RPS"`` or the | ||
input is not a meta-tensor or has no ``"space"`` item, the | ||
value ``(('L', 'R'), ('P', 'A'), ('I', 'S'))`` is used. If not | ||
``None``, the provided value is always used and the ``"space"`` | ||
metadata item (if any) of the input is ignored. | ||
allow_missing_keys: don't raise exception if key is missing. | ||
lazy: a flag to indicate whether this transform should execute lazily or not. | ||
Defaults to False | ||
|
Uh oh!
There was an error while loading. Please reload this page.