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

Merge prep_cmr with image_transform plus tests #123

Merged
merged 8 commits into from Apr 28, 2021
Merged

Merge prep_cmr with image_transform plus tests #123

merged 8 commits into from Apr 28, 2021

Conversation

shuo-zhou
Copy link
Member

Fixes card.

Description

  • Merging functions in kale.prepdata.prep_cmr with kale.prepdata.image_transform.
  • Adding tests for image transform.

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.
  • In-line docstrings updated.
  • Source for documentation at docs manually updated for new API.

@shuo-zhou shuo-zhou added refactor Refactor of existing code tests Tests and coverage labels Apr 26, 2021
@shuo-zhou shuo-zhou added this to In progress in v0.1.0 via automation Apr 26, 2021
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2021

Codecov Report

Merging #123 (22fb16a) into main (2b62c90) will increase coverage by 1.89%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #123      +/-   ##
==========================================
+ Coverage   22.60%   24.50%   +1.89%     
==========================================
  Files          38       37       -1     
  Lines        3012     2975      -37     
==========================================
+ Hits          681      729      +48     
+ Misses       2331     2246      -85     
Impacted Files Coverage Δ
kale/prepdata/image_transform.py 90.54% <100.00%> (+17.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b62c90...22fb16a. Read the comment docs.

with pytest.raises(Exception):
reg_img_stack(images, coords[1:, :])
images_reg, max_dist = reg_img_stack(images, coords)
testing.assert_allclose(images_reg, images)
Copy link
Member

Choose a reason for hiding this comment

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

I did not get fully your logic here and can only guess. How did you make the registered images to be close to the original? I though you randomly perturbed the coordinates of the n-1 (9) images. After registration, they are close. Is it because the random noise is of small value compared to the coords? You can explain to me with a voice message in WeChat or Skype. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the poor readability. Yes, the random noise to source coordinates are in (0, 1), which are small values compared to the image size. So the images after registration should be close to the original ones. The values in list max_dist are very small. Maybe we can test whether max_dist + 1 is close to a ones vector (+1 for avoiding inf relative difference). Because we do not have real images and landmark coordinates for testing here, this is probably the easiest way for me to test the function. I will add more comments to improve the readability.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the clarification. That's helpful! Ready to merge.

if i == dst_id:
continue
else:
# epts = landmarks.iloc[i, 1:].values
Copy link
Member

Choose a reason for hiding this comment

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

Still useful? Otherwise, remove.

Copy link
Member Author

Choose a reason for hiding this comment

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

will remove it


from kale.prepdata.image_transform import mask_img_stack, reg_img_stack, rescale_img_stack

gait = loadmat("tests/test_data/gait_gallery_data.mat")
Copy link
Member

Choose a reason for hiding this comment

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

Smart to use gait to test image stacks. Good reuse.

@haipinglu haipinglu changed the title Merge prep_cmr with image_transform Merge prep_cmr with image_transform plus tests Apr 28, 2021
@haipinglu haipinglu merged commit d6fbe21 into main Apr 28, 2021
v0.1.0 automation moved this from In progress to Done Apr 28, 2021
@haipinglu haipinglu deleted the cmr_prep branch April 28, 2021 06:04
@github-actions github-actions bot mentioned this pull request Apr 29, 2021
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor of existing code tests Tests and coverage
Projects
No open projects
v0.1.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants