Skip to content

2562 Add support to not save data in a separate folder#2576

Merged
ericspod merged 7 commits intoProject-MONAI:devfrom
Nic-Ma:2562-enhance-save-folder
Jul 12, 2021
Merged

2562 Add support to not save data in a separate folder#2576
ericspod merged 7 commits intoProject-MONAI:devfrom
Nic-Ma:2562-enhance-save-folder

Conversation

@Nic-Ma
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma commented Jul 12, 2021

Fixes #2562 .

Description

According to user's feedback, this PR enhanced the nifti_saver and png_saver to optionally save every image in a separate folder or not.

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@Nic-Ma Nic-Ma requested review from ericspod, rijobro and wyli July 12, 2021 10:09
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jul 12, 2021

/black

ericspod
ericspod reviewed Jul 12, 2021
View reviewed changes
@ericspod
Copy link
Copy Markdown
Member

ericspod commented Jul 12, 2021

I would suggest the default value should be False for this option to preserve existing behaviour, I personally would probably tend not to use this feature.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jul 12, 2021

I would suggest the default value should be False for this option to preserve existing behaviour, I personally would probably tend not to use this feature.

Hi @ericspod ,

Thanks for your review and comments!
Actually, the current existing behavior in MONAI v0.6 equals True here, that's why users asked to support saving all images in the same folder.

Thanks.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jul 12, 2021

/black

@Nic-Ma Nic-Ma requested a review from ericspod July 12, 2021 11:05
@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Jul 12, 2021

Hi @ericspod ,

Thanks for your review.
I updated the PR according to your comments.
Could you please help review again?

Thanks in advance.

wyli
wyli reviewed Jul 12, 2021
View reviewed changes
monai/data/utils.py
else:
output = os.path.join(subfolder_path, filename)
if separate_folder:
output = os.path.join(output, filename)
Copy link
Copy Markdown
Contributor

@wyli wyli Jul 12, 2021

Choose a reason for hiding this comment

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

not sure if this filename should also have the postfix?

Copy link
Copy Markdown
Member

@ericspod ericspod Jul 12, 2021

Choose a reason for hiding this comment

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

That will be used in the next lines for the subfolders that go in the folder referred to as output here.

Copy link
Copy Markdown
Contributor Author

@Nic-Ma Nic-Ma Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @wyli @ericspod ,

I think you mean to save as: "output/image_seg/image_seg.nii", right? The existing bahvior in MONAI is "output/image/image_seg.nii".
Maybe let me change the original logic to "output/image_seg/image_seg.nii"? Seems there is not drawback?

Thanks.

@ericspod ericspod merged commit daf2c4f into Project-MONAI:dev Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wyli wyli wyli left review comments

@ericspod ericspod ericspod approved these changes

@rijobro rijobro Awaiting requested review from rijobro

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Save segmentation results in the same folder

3 participants

@Nic-Ma @ericspod @wyli