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

5809 hedjitter #5994

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

5809 hedjitter #5994

wants to merge 11 commits into from

Conversation

Ycblue
Copy link

@Ycblue Ycblue commented Feb 14, 2023

Fixes #5809 .
@drbeh sorry for the long wait. Some things came inbetween and I couldn't work on the feature till now. I asked the original author of the code for permission and he added an Apache 2.0 license to his repo.
Btw, what is the correct route of communication? Should we communicate over pull requests or in the comments of the issue?

Description

Implementation of HEDJitter in MONAI framework.
The proposed transform randomly perturbes HED color space values of an RGB image.

Added HEDJitter class to pathology.transforms.stain.array.py
Added HEDJitterd class to pathology.transforms.stain.dictionary.py
Passed codeformat and autofix.
Still designing unit tests.

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

@bhashemian
Copy link
Member

bhashemian commented Feb 14, 2023

Hi @Ycblue,
Thanks for your contribution. We can continue our communication related to this PR here. Please try to fix the errors that automatic checks are reporting but let me know if you need help to understand or resolve any of them.
After all the checks are passed, we can start the review process.

@bhashemian bhashemian self-requested a review February 14, 2023 21:28
@bhashemian
Copy link
Member

bhashemian commented Feb 21, 2023

Hi @Ycblue, I resolved the conflict that your PR had with the base and merged it. Please pull before making other changes and let us know if you need any help with resolving the checking errors.

@Ycblue
Copy link
Author

Ycblue commented Mar 29, 2023

Hi @drbeh, sorry for being absent for so long.
I'm trying to to some unit tests with a test image. The documentations say to host them here, but I can't figure out how to upload the images.
Some guidance would be much appreciated!
Best, Yu-Chia

@bhashemian
Copy link
Member

Hi @drbeh, sorry for being absent for so long.
I'm trying to to some unit tests with a test image. The documentations say to host them here, but I can't figure out how to upload the images.
Some guidance would be much appreciated!
Best, Yu-Chia

Hi @Ycblue, thanks for adding unit tests. May I know what this image is and what is the license for this image?

@Ycblue
Copy link
Author

Ycblue commented Mar 29, 2023

Hi,
I wanted to start out with the sample image provided in the reference repository.
The repo operates under a Apache 2.0 license, so it would be okay to use that image as well?

@bhashemian
Copy link
Member

@Ycblue
Is this the image that you are referring to?

I did not find anything specific about the license of this data but as it is release as part of the repo with Apache 2.0, I think its license should be the same.

For now, maybe it's better to use the URL from their repo (https://github.com/gatsby2016/Augmentation-PyTorch-Transforms/raw/master/data/10-05074_353_49_8178.png) to write the unittests and check and verify everything works, then I can help you to either use one of existing images in monai or upload it to monai-extra-test-data. What do you think?

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.

HEDJitter Transform
2 participants