Skip to content

Conversation

@kbressem
Copy link
Contributor

@kbressem kbressem commented May 11, 2022

Signed-off-by: kbressem kenobressem@gmail.com

Fixes #4238.

Description

This PR proposes the NrrdReader class to correctly read .nrrd and .seg.nrrd files.

Status

Ready for review

Files changes/created

monai/data/image_reader.py --> add NrrdReader and NrrdImage
monai/data/__init__.py --> add NrrdReader to imports
monai/transforms/io/array.py --> add NrrdReader to imports and "nrrdreader" to SUPPORTED_READERS
monai/config/deviceconfig.py --> add pynrrd to get_optional_config_values

tests/test_init_reader.py  --> add test for NrrdReader
tests/min_tests.py  --> exclude tests for NrrdReader
requirements-dev.txt --> add pynrrd

FIles created

tests/test_nrrd_reader.py --> tests for NrrdReader

Types of changes

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

ahatamiz and others added 13 commits May 12, 2022 11:41
* add swin_unetr model

Signed-off-by: kbressem <kenobressem@gmail.com>
* [DLMED] update to 22.04

Signed-off-by: Nic Ma <nma@nvidia.com>

* fixes unit test tests.test_lr_finder

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

* test new_empty

Signed-off-by: Wenqi Li <wenqil@nvidia.com>

Co-authored-by: Wenqi Li <wenqil@nvidia.com>
Co-authored-by: Wenqi Li <831580+wyli@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* implement the base class

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add unittest

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* autofix

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* switch to call apex directly

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* uncomment unittest

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* add apex install link in docstring

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* add channels_last_3d test case

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* rewrite types

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* change types

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* add docstrings

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Wenqi Li <831580+wyli@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* Update dice.py

reduce redundant operations in DiceFocalLoss, initially caused oom

Signed-off-by: Ryan Clanton <55164720+ryancinsight@users.noreply.github.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Signed-off-by: Ryan Clanton <55164720+ryancinsight@users.noreply.github.com>

* [MONAI] python code formatting

Signed-off-by: monai-bot <monai.miccai2019@gmail.com>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: monai-bot <monai.miccai2019@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* Make all transforms optional

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update wsireader tests

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Remove optional from PersistentDataset and its derivatives

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add unittests for cache without transform

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add default replace_rate

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add default value

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Set default replace_rate to 0.1

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update metadata to include path

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Adds SmartCachePatchWSIDataset

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add unittests for SmartCachePatchWSIDataset

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update references

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update docs

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Remove smart cache

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Remove unused imports

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add path metadata for OpenSlide

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update metadata to be unified across different backends

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Update wsi metadata for multi wsi objects

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>

* Add unittests for wsi metadata

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* replace modules

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* replace_module -> replace_modules

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>

* fix

Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: Can Zhao <canz@nvidia.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* reproduce issue

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* remove 22.01 02

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* remove other workflows

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* run on pull request

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* remove sleep

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* test single layer forward

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* add has_nvfuser

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* add check within factory

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* revert to original cron.yml

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix old pt issue

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

* change to return directly if no cuda

Signed-off-by: Yiheng Wang <vennw@nvidia.com>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
* Update to bundle specifiation

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Adding description in spec discussing the saved Torchscript object's file storage behaviour, and tweaking ckpt_export to add .json extension

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>

* Annotating optional bundle files

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>

* Adjusted ckpt_export test

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>

* Fix

Signed-off-by: Eric Kerfoot <eric.kerfoot@kcl.ac.uk>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
for more information, see https://pre-commit.ci

Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
kbressem added 9 commits May 12, 2022 19:54
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
make flake8 happy

Signed-off-by: kbressem <kenobressem@gmail.com>
Changed NrrdImage to dataclass

Signed-off-by: kbressem <kenobressem@gmail.com>
@kbressem kbressem changed the title [WIP] Implement NrrdReader Implement NrrdReader May 13, 2022
@kbressem kbressem marked this pull request as ready for review May 13, 2022 09:57
@Nic-Ma
Copy link
Contributor

Nic-Ma commented May 13, 2022

Hi @kbressem ,

Thanks very much for your contribution.
There are several places need to add the optional package, you can search pandas as the reference.

Thanks in advance.

@kbressem
Copy link
Contributor Author

Hi @Nic-Ma

I've searched for optional_import("pandas"), pandas, 'optional_import("nibabel")', nibabel and nib in all scripts. Then I went through the scripts, to check if adding nrrd as optional import would be usefull.

pandas was imported in:

  • monai/data/iterable_dataset.py No image reader is called. I believe nrrd would not be used
  • monai/data/utils.py pandas is used to convert tables to dicts. nrrd would not be used
  • monai/data/dataset.py No image reader is called. I believe nrrd would not be used
  • monai/config/deviceconfig.py I've added nrrd to get_optional_config_values

nibabel was imported in:

  • monai/transforms/spatial/dictionary.py nibabel is used in Orientationd only. nrrd would not be used.
  • monai/transforms/spatial/array.py nibabel is used in SpatialResample only, nrrd would not be used.
  • monai/transforms/io/array.py added nrrd as optional import and "nrrdreader" to SUPPORTED_READERS
  • monai/data/image_reader.py obviously added nrrd :)
  • monai/data/utils.py nibabel is used to reorient spatial axes. nrrd would not be used
  • monai/data/nifti_writer.py nrrd would not be used
  • monai/data/image_writer.py Currently there is no NrrdWriter. We are working on one in MONAI-Label and would like to contribute in a future PR.
  • monai/utils/module.py adding nrrd here would not make sense
  • monai/config/deviceconfig.py I've added nrrd to get_optional_config_values

I did not find any new instance where importing nrrd would be useful. Did I miss something?

@Nic-Ma
Copy link
Contributor

Nic-Ma commented May 13, 2022

kbressem and others added 2 commits May 13, 2022 13:15
Signed-off-by: kbressem <kenobressem@gmail.com>
@kbressem
Copy link
Contributor Author

Hi @kbressem ,

Sorry maybe I didn't make it clear. I mean you need to add new optional import into some project config files, referring to pandas. For example: https://github.com/Project-MONAI/MONAI/blob/dev/setup.cfg https://github.com/Project-MONAI/MONAI/blob/dev/docs/requirements.txt https://github.com/Project-MONAI/MONAI/blob/dev/requirements-dev.txt https://github.com/Project-MONAI/MONAI/blob/dev/environment-dev.yml https://github.com/Project-MONAI/MONAI/blob/dev/docs/source/installation.md https://github.com/Project-MONAI/MONAI/blob/dev/monai/config/deviceconfig.py

Thanks.

Thank you for the clarification. I have added pynrrd to all required files.

@wyli
Copy link
Contributor

wyli commented May 13, 2022

this looks nice, one additional test that is worth adding is to check the consistency of the writer/reader...

ideally we should have a TestLoadSaveNrrd, structured like this one:

class TestLoadSavePNG(unittest.TestCase):
def setUp(self):
self.test_dir = tempfile.mkdtemp()
def tearDown(self):
shutil.rmtree(self.test_dir, ignore_errors=True)
def png_rw(self, test_data, reader, writer, dtype, resample=True):
test_data = test_data.astype(dtype)
ndim = len(test_data.shape) - 1
for p in TEST_NDARRAYS:
output_ext = ".png"
filepath = f"testfile_{ndim}d"
saver = SaveImage(
output_dir=self.test_dir, output_ext=output_ext, resample=resample, separate_folder=False, writer=writer
)
saver(p(test_data), {"filename_or_obj": f"{filepath}.png", "spatial_shape": (6, 8)})
saved_path = os.path.join(self.test_dir, filepath + "_trans" + output_ext)
self.assertTrue(os.path.exists(saved_path))
loader = LoadImage(reader=reader)
data, meta = loader(saved_path)
if meta["original_channel_dim"] == -1:
_test_data = moveaxis(test_data, 0, -1)
else:
_test_data = test_data[0]
assert_allclose(data, _test_data)
@parameterized.expand(itertools.product([PILReader, ITKReader], [PILWriter, ITKWriter]))
def test_2d(self, reader, writer):
test_data = np.arange(48, dtype=np.uint8).reshape(1, 6, 8)
self.png_rw(test_data, reader, writer, np.uint8)
@parameterized.expand(itertools.product([PILReader, ITKReader], ["monai.data.PILWriter", ITKWriter]))
def test_rgb(self, reader, writer):
test_data = np.arange(48, dtype=np.uint8).reshape(3, 2, 8)
self.png_rw(test_data, reader, writer, np.uint8, False)

the goal is to test whether the nrrd file created by monai's ITKWriter is readable by monai's NrrdReader for some basic (non multichannel?)cases.

kbressem added 4 commits May 13, 2022 14:19
…n header, it is assumed to be LPS and converted to RAS. If space is defined and not LPS, nothing is done to prevent wrong conversions.

Signed-off-by: kbressem <kenobressem@gmail.com>
…ITKWriter can be loaded again. 2D and 3D files with no channels are tested

Signed-off-by: kbressem <kenobressem@gmail.com>
Signed-off-by: kbressem <kenobressem@gmail.com>
@kbressem
Copy link
Contributor Author

kbressem commented May 13, 2022

the goal is to test whether the nrrd file created by monai's ITKWriter is readable by monai's NrrdReader for some basic (non multichannel?)cases.

I have added a TestLoadSaveNrrd class to test_image_rw.py which tests writing and reading 2d and 3d images. At the moment this only writes using ITKWriter, in the future an NrrdWriter, which is currently developed at MONAILabel might be used: Project-MONAI/MONAILabel#779

@wyli
Copy link
Contributor

wyli commented May 13, 2022

/build

@wyli wyli enabled auto-merge (squash) May 13, 2022 16:56
@wyli wyli merged commit 0c243b9 into Project-MONAI:dev May 13, 2022
@kbressem kbressem deleted the 4238-nrrd-reader branch May 13, 2022 17:49
@Nic-Ma Nic-Ma mentioned this pull request May 20, 2022
7 tasks
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.

Implement NrrdReader for nrrd and seg.nrrd files

10 participants