#6188 Allow user to define own `FolderLayout` in `SaveImage` and `SaveImaged` transforms #6213

MathijsdeBoer · 2023-03-21T13:39:16Z

Improves issues discussed in #6188 .

Description

As I found out in #6188, SaveImage and SaveImaged provide a fairly rigid way to define the output path. I've made some changes that allow the user to more easily define their own FolderLayout derivative, and pass it to these transforms.
Additionally, I've added a boolean that adds a key "saved_to" to the metadata, so the user can more easily assess where their output actually went.

Finally, and most optionally, I've prepended "meta" to the MetaTensor.__repr__ and MetaTensor.__str__ outputs in an effort to more clearly show to the user that the printed tensor has special properties. This change was made in its own commit, which can be left out if undesired.

Other small changes to documentation formatting, mainly changing:

"""
    ...
    some documentation `code` more docs
    ...
"""

to

"""
    ...
    some documentation ``code`` more docs
    ...
"""

To format these correctly in the docs. Seemed like some people assumed the single backticks would format as code, as opposed to italics.

Test results

./runtests.sh -f -u --net --coverage
results: 1 failure, 2 errors
The failure was related to me changing the MetaTensor.__repr__, fixed the test in a separate commit

./runtests.sh --quick --unittests --disttests
In progress

Types of changes

Non-breaking change (fix or new feature that would not break existing functionality).
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.

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

…d for custom implementations Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Add `savepath_in_metadict`, to include the location of the output image in the metadata Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

monai/data/meta_tensor.py

…d for custom implementations Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Add `savepath_in_metadict`, to include the location of the output image in the metadata Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

This reverts commit 145fca0.

This reverts commit 9e1dcd4.

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

…__` for easier identification (#6214) Discussed in #6213 ### Description Prepends `"meta"` to the `MetaTensor.__repr__` and `MetaTensor.__str__` output so printing a MetaTensor does not look the exact same as a regular `torch.Tensor`. I don't expect this change to cause any breaks, with me running the risk of invoking [xkcd 1172](https://xkcd.com/1172/). --- 1 failure in `./runtests.sh -f -u --net --coverage`: ```text ====================================================================== FAIL: test_values (tests.test_tciadataset.TestTciaDataset) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\PythonProjects\MONAI-SaveImageFormatting\tests\test_tciadataset.py", line 72, in test_values self.assertTrue( AssertionError: False is not true ``` ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] New tests added to cover the changes. - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. --------- Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

wyli · 2023-03-22T16:52:27Z

could you please update the test case in tests/test_save_imaged.py

MathijsdeBoer · 2023-03-22T18:01:12Z

Will do! (Though I might have accidentally clicked a wrong button on this page)

Edit: Nope, the button did what I expected it to do!

…yout Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

MathijsdeBoer · 2023-03-22T21:04:42Z

I hope it's okay if I didn't rerun the full test suite, and only ran the edited tests. It takes about an hour to do on my work PC, which is considerably more powerful than my personal PC that I used for these last few commits.

monai/transforms/io/array.py

KumoLiu

Thanks for the contributions! Few comments in line.
You could also run ./runtests.sh --autofix for auto style fixes.

monai/transforms/io/array.py

monai/transforms/io/dictionary.py

monai/transforms/io/array.py

monai/transforms/io/dictionary.py

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Mathijs de Boer <8137653+MathijsdeBoer@users.noreply.github.com>

Formatting fix in test_save_imaged.py Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

# Conflicts: # monai/transforms/io/array.py # monai/transforms/io/dictionary.py

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

# Conflicts: # tests/test_save_imaged.py

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

wyli · 2023-03-23T20:41:28Z

/build

MathijsdeBoer · 2023-03-23T22:41:47Z

Haha, now I'm starting to get slightly embarrased about the amount of small commits I have to make because I thought I had ran every test, but forgot that one, I'll get to work on fixing that mypy error.

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

wyli

thanks, I think the current PR's implementation is ok for now, another idea is to set the path string as a property of SaveImage, then after saving we can get the path by saver.last_saved_filename cc @ericspod

wyli · 2023-03-24T19:14:01Z

/build

…aTensor.__str__` for easier identification (Project-MONAI#6214) Discussed in Project-MONAI#6213 ### Description Prepends `"meta"` to the `MetaTensor.__repr__` and `MetaTensor.__str__` output so printing a MetaTensor does not look the exact same as a regular `torch.Tensor`. I don't expect this change to cause any breaks, with me running the risk of invoking [xkcd 1172](https://xkcd.com/1172/). --- 1 failure in `./runtests.sh -f -u --net --coverage`: ```text ====================================================================== FAIL: test_values (tests.test_tciadataset.TestTciaDataset) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\PythonProjects\MONAI-SaveImageFormatting\tests\test_tciadataset.py", line 72, in test_values self.assertTrue( AssertionError: False is not true ``` ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] New tests added to cover the changes. - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. --------- Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

…age` and `SaveImaged` transforms (Project-MONAI#6213) Improves issues discussed in Project-MONAI#6188 . ### Description As I found out in Project-MONAI#6188, `SaveImage` and `SaveImaged` provide a fairly rigid way to define the output path. I've made some changes that allow the user to more easily define their own `FolderLayout` derivative, and pass it to these transforms. Additionally, I've added a boolean that adds a key `"saved_to"` to the metadata, so the user can more easily assess where their output actually went. Finally, and most optionally, I've prepended `"meta"` to the `MetaTensor.__repr__` and `MetaTensor.__str__` outputs in an effort to more clearly show to the user that the printed tensor has special properties. This change was made in its own commit, which can be left out if undesired. Other small changes to documentation formatting, mainly changing: ```text """ ... some documentation `code` more docs ... """ ``` to ```text """ ... some documentation ``code`` more docs ... """ ``` To format these correctly in the docs. Seemed like some people assumed the single backticks would format as `code`, as opposed to _italics_. --- ### Test results `./runtests.sh -f -u --net --coverage` results: 1 failure, 2 errors The failure was related to me changing the `MetaTensor.__repr__`, fixed the test in a separate commit `./runtests.sh --quick --unittests --disttests` In progress ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. - [x] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

…aTensor.__str__` for easier identification (Project-MONAI#6214) Discussed in Project-MONAI#6213 ### Description Prepends `"meta"` to the `MetaTensor.__repr__` and `MetaTensor.__str__` output so printing a MetaTensor does not look the exact same as a regular `torch.Tensor`. I don't expect this change to cause any breaks, with me running the risk of invoking [xkcd 1172](https://xkcd.com/1172/). --- 1 failure in `./runtests.sh -f -u --net --coverage`: ```text ====================================================================== FAIL: test_values (tests.test_tciadataset.TestTciaDataset) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\PythonProjects\MONAI-SaveImageFormatting\tests\test_tciadataset.py", line 72, in test_values self.assertTrue( AssertionError: False is not true ``` ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] New tests added to cover the changes. - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. --------- Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

…age` and `SaveImaged` transforms (Project-MONAI#6213) Improves issues discussed in Project-MONAI#6188 . ### Description As I found out in Project-MONAI#6188, `SaveImage` and `SaveImaged` provide a fairly rigid way to define the output path. I've made some changes that allow the user to more easily define their own `FolderLayout` derivative, and pass it to these transforms. Additionally, I've added a boolean that adds a key `"saved_to"` to the metadata, so the user can more easily assess where their output actually went. Finally, and most optionally, I've prepended `"meta"` to the `MetaTensor.__repr__` and `MetaTensor.__str__` outputs in an effort to more clearly show to the user that the printed tensor has special properties. This change was made in its own commit, which can be left out if undesired. Other small changes to documentation formatting, mainly changing: ```text """ ... some documentation `code` more docs ... """ ``` to ```text """ ... some documentation ``code`` more docs ... """ ``` To format these correctly in the docs. Seemed like some people assumed the single backticks would format as `code`, as opposed to _italics_. --- ### Test results `./runtests.sh -f -u --net --coverage` results: 1 failure, 2 errors The failure was related to me changing the `MetaTensor.__repr__`, fixed the test in a separate commit `./runtests.sh --quick --unittests --disttests` In progress ### Types of changes  - [x] Non-breaking change (fix or new feature that would not break existing functionality). - [x] Integration tests passed locally by running `./runtests.sh -f -u --net --coverage`. - [x] Quick tests passed locally by running `./runtests.sh --quick --unittests --disttests`. - [x] In-line docstrings updated. - [x] Documentation updated, tested `make html` command in the `docs/` folder. --------- Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Mathijs de Boer and others added 5 commits March 21, 2023 13:26

Add the "meta" prefix to MetaTensor __repr__ and __str__

145fca0

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Add the FolderLayoutBase class to serve as an abstract class to exten…

57c23ee

…d for custom implementations Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Use replaceable FolderLayout in SaveImage and SaveImaged

7e72b85

Add `savepath_in_metadict`, to include the location of the output image in the metadata Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Fix accidentally introduced circular imports.

b6ee390

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Fix MetaTensor tests

9e1dcd4

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

wyli reviewed Mar 21, 2023

View reviewed changes

monai/data/meta_tensor.py Outdated Show resolved Hide resolved

Mathijs de Boer added 6 commits March 21, 2023 14:54

Add the FolderLayoutBase class to serve as an abstract class to exten…

0fe4034

…d for custom implementations Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Use replaceable FolderLayout in SaveImage and SaveImaged

b9787c7

Add `savepath_in_metadict`, to include the location of the output image in the metadata Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Fix accidentally introduced circular imports.

6e4b757

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Merge remote-tracking branch 'origin/dev' into dev

c65aed3

Revert "Add the "meta" prefix to MetaTensor __repr__ and __str__"

744bbd1

This reverts commit 145fca0.

Revert "Fix MetaTensor tests"

2e058f6

This reverts commit 9e1dcd4.

MathijsdeBoer mentioned this pull request Mar 21, 2023

#6213 Prepend "meta" to MetaTensor.__repr__ and MetaTensor.__str__ for easier identification #6214

Merged

5 tasks

Pass MyPy checks

f51fdc5

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

wyli requested review from yiheng-wang-nv, Nic-Ma, ericspod and KumoLiu March 22, 2023 16:51

Merge branch 'dev' into dev

c0d7a6e

Mathijs de Boer added 2 commits March 22, 2023 22:01

Small documentation update, to signify effects of including folder_la…

32701a4

…yout Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Added unit tests for folder_layout and savepath_in_metadict

6ad0984

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Nic-Ma reviewed Mar 23, 2023

View reviewed changes

monai/transforms/io/array.py Show resolved Hide resolved

monai/transforms/io/array.py Outdated Show resolved Hide resolved

KumoLiu reviewed Mar 23, 2023

View reviewed changes

MathijsdeBoer and others added 3 commits March 23, 2023 11:51

Documentation formatting by KumoLiu

0907cdf

Co-authored-by: YunLiu <55491388+KumoLiu@users.noreply.github.com> Signed-off-by: Mathijs de Boer <8137653+MathijsdeBoer@users.noreply.github.com>

Documentation language fix

6dcc322

Formatting fix in test_save_imaged.py Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Merge remote-tracking branch 'origin/dev' into dev

b3391ca

# Conflicts: # monai/transforms/io/array.py # monai/transforms/io/dictionary.py

Mathijs de Boer and others added 5 commits March 23, 2023 16:33

Use unused variables

9f9a7d2

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Use unused variables

0d1a99c

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Merge remote-tracking branch 'origin/dev' into dev

fe0d4d9

# Conflicts: # tests/test_save_imaged.py

Run autoformat. For missing one comma.

6cb0344

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Merge branch 'dev' into dev

f79b2ad

Mathijs de Boer added 2 commits March 24, 2023 00:51

Fix mypy error

047a62a

Signed-off-by: Mathijs de Boer <m.deboer-41@umcutrecht.nl>

Merge remote-tracking branch 'origin/dev' into dev

506c31e

wyli approved these changes Mar 24, 2023

View reviewed changes

Merge branch 'dev' into dev

6a70065

wyli enabled auto-merge (squash) March 24, 2023 19:14

wyli merged commit c885460 into Project-MONAI:dev Mar 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#6188 Allow user to define own `FolderLayout` in `SaveImage` and `SaveImaged` transforms #6213

#6188 Allow user to define own `FolderLayout` in `SaveImage` and `SaveImaged` transforms #6213

MathijsdeBoer commented Mar 21, 2023 •

edited

wyli commented Mar 22, 2023

MathijsdeBoer commented Mar 22, 2023 •

edited

MathijsdeBoer commented Mar 22, 2023

KumoLiu left a comment •

edited

wyli commented Mar 23, 2023

MathijsdeBoer commented Mar 23, 2023

wyli left a comment •

edited

wyli commented Mar 24, 2023

#6188 Allow user to define own FolderLayout in SaveImage and SaveImaged transforms #6213

#6188 Allow user to define own FolderLayout in SaveImage and SaveImaged transforms #6213

Conversation

MathijsdeBoer commented Mar 21, 2023 • edited

Description

Test results

Types of changes

wyli commented Mar 22, 2023

MathijsdeBoer commented Mar 22, 2023 • edited

MathijsdeBoer commented Mar 22, 2023

KumoLiu left a comment • edited

Choose a reason for hiding this comment

wyli commented Mar 23, 2023

MathijsdeBoer commented Mar 23, 2023

wyli left a comment • edited

Choose a reason for hiding this comment

wyli commented Mar 24, 2023

#6188 Allow user to define own `FolderLayout` in `SaveImage` and `SaveImaged` transforms #6213

#6188 Allow user to define own `FolderLayout` in `SaveImage` and `SaveImaged` transforms #6213

MathijsdeBoer commented Mar 21, 2023 •

edited

MathijsdeBoer commented Mar 22, 2023 •

edited

KumoLiu left a comment •

edited

wyli left a comment •

edited