Skip to content
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

Extra transform examples #3056

Merged
merged 23 commits into from
Oct 4, 2021
Merged

Conversation

rijobro
Copy link
Contributor

@rijobro rijobro commented Oct 1, 2021

Description

Adds the majority of examples for transforms.

new transforms.html can be seen here (I had to add the .txt extension, so remove that):

https://github.com/Project-MONAI/MONAI/files/7266593/transforms.html.txt

Alternatively, you can see the images (out of context) here:

https://github.com/Project-MONAI/DocImages/tree/main/transforms

Status

Ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Documentation updated, tested make html command in the docs/ folder.

rijobro and others added 18 commits September 27, 2021 16:52
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
Signed-off-by: Richard Brown <33289025+rijobro@users.noreply.github.com>
@rijobro rijobro requested review from wyli and Nic-Ma October 1, 2021 10:51
@rijobro
Copy link
Contributor Author

rijobro commented Oct 1, 2021

@wyli I tried modifying the docs/conf.py to include this:

# If there is a + in the version, that means that we're not on a tagged version.
# In this case, we'll use the URL to the `main` branch. If we're on a clean version,
# then use that tag.
if "+" in monai.__version__:
    doc_image_url = "https://github.com/Project-MONAI/DocImages/raw/main/"
else:
    doc_image_url = "https://raw.githubusercontent.com/Project-MONAI/DocImages/" + monai.__version__ + "/"


rst_epilog = f"""
.. |DocImageURL| replace:: {doc_image_url}
"""

and then |DocImageURL| can be accessed inside the .rst files. Unfortunately, it seems that in sphinx, you can't do variable substitution inside of another directive. So .. image:: |DocImageURL|transforms/BorderPad.png is taken as a literal, and the variable is not substituted. See here for same problem, which doesn't look its easy to side-step...

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 1, 2021

Hi @rijobro ,

@wyli and I are in the national holiday, will try to review ASAP soon.

Thanks.

Copy link
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.

Thanks very much for the great work here!
I checked the preview, it looks fantastic.
I have some minor comments:

  1. The example of RandCropByPosNegLabel seems not correct(it's not 8 samples, and why some black regions?):
    image
  2. Could you please also add example image to KeepLargestConnectedComponent? I think it's an important transform and not very straight-forward for users.

Thanks.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 2, 2021

BTW, maybe I missed some context discussion, may I know why the dict transforms use colorful images and array transforms use black & white images?

Thanks.

Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

thanks, it looks good to me.

@wyli wyli enabled auto-merge (squash) October 4, 2021 06:18
@wyli wyli merged commit a0aac6a into Project-MONAI:dev Oct 4, 2021
@rijobro
Copy link
Contributor Author

rijobro commented Oct 4, 2021

@Nic-Ma I used colour for the dictionary transforms because they have the segmentations overlayed. The underlying MR image is black and white in both cases. Let me know if you find it unclear and we can discuss something better.

@rijobro rijobro deleted the extra_transform_example branch October 4, 2021 10:43
@rijobro
Copy link
Contributor Author

rijobro commented Oct 4, 2021

@Nic-Ma , and you're right. There's something wrong with all the num_samples=8, I'll check that.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 4, 2021

@Nic-Ma I used colour for the dictionary transforms because they have the segmentations overlayed. The underlying MR image is black and white in both cases. Let me know if you find it unclear and we can discuss something better.

Got it, thanks!

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Oct 4, 2021

@Nic-Ma , and you're right. There's something wrong with all the num_samples=8, I'll check that.

Sure, please also take a look other crop-samples transforms, maybe they are also not correct.

Thanks.

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.

4 participants