Cleaned PR for Visualization Modules to Assess the quality of Registration Qualitatively. #1606

parichit · 2018-07-31T17:45:33Z

This work was done as part of the Google Summer of Code (GSOC-2018) DIPY project titled 'DIPY Workflow(s) and Quality Assurance'

This work pertains to developing a workflow to visualize the quality of the registered images by creating a mosaic or the animation or both depending on the requirement. The plane to be visualized (Axial, Coronal or sagittal) can be easily controlled through the command line parameters.

This is a cleaned PR for the visualization modules developed to support the Image Registration Workflow(s). The PR1595 must not be merge now. Instead, this should be merged after making the changes in the core.py file for the GIF color range issues. The exact order of merging the PR's should be

PR1604 (affine registration workflow) -> PR1605 (apply transformation workflow) - > PR1606 (visualisation)

The link to the older PR (with all the conversations, commit history, improvements based on community feedback can be seen below):
#1595

pep8speaks · 2018-07-31T17:45:37Z

Hello @parichit, Thank you for updating !

In the file dipy/workflows/core.py, following are the PEP8 issues :

Line 426:31: W292 no newline at end of file

Comment last updated on August 13, 2018 at 21:22 Hours UTC

…rkflow. 1) Committing to this branch all the code from the affine_registration branch (PR 1581) 2) New PR should be opened for this branch.

…ile for test cases.

… dipy_align_affine.

…of mass' transformation rather than 'translation' when the progressive flag is off.

…ix the PEP8 issue.

…ass.

…value of distance metric) to validate the tests by travis.

…ilarity metric in the optimize() function of the imaffine.py file. 2) Changed the parameter documentation in the run() method of the alignpy file for improved view on the command line.

codecov-io · 2018-08-13T18:14:39Z

Codecov Report

Merging #1606 into master will decrease coverage by 0.63%.
The diff coverage is 42.39%.

@@            Coverage Diff             @@
##           master    #1606      +/-   ##
==========================================
- Coverage   87.34%   86.71%   -0.64%     
==========================================
  Files         246      249       +3     
  Lines       31811    32344     +533     
  Branches     3451     3519      +68     
==========================================
+ Hits        27785    28046     +261     
- Misses       3204     3472     +268     
- Partials      822      826       +4

Impacted Files	Coverage Δ
dipy/viz/regtools.py	`32.5% <0%> (-0.56%)`	⬇️
dipy/io/image.py	`100% <100%> (ø)`	⬆️
dipy/align/imaffine.py	`91.84% <100%> (+0.04%)`	⬆️
dipy/workflows/align.py	`92.63% <100%> (-1.12%)`	⬇️
dipy/workflows/core.py	`17.22% <17.22%> (ø)`
dipy/workflows/vis_registration.py	`24.36% <24.36%> (ø)`
dipy/workflows/tests/test_vis.py	`93.1% <93.1%> (ø)`
dipy/workflows/tests/test_align.py	`98.09% <97.77%> (+7.18%)`	⬆️
... and 3 more

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 5a6aa5a...73fc149. Read the comment docs.

…1598.

…apply_transform. 2) Changed the body of 'rigid()' function in align.py to call the center_of_mass function when the progressive flag is off.

…t from the Syn_Registration flow. 2) Made the changes in the command line wrapper to use the changed name for the workflow.

…orm() class.

… decreased the decimal precision to 1 decimal place for validating the travis tests.

…ng in the path of ApplyAffineWorkflow().

2) Added a utility funciton in the vis_register.py to check the slice type and raise the error.

… the numpy.all() method on the array.

Borda

there is missing documentation for several functions and some are incomplete
overall I feel that the PR is too long and complex 8-)

Borda · 2019-05-16T07:55:49Z

dipy/align/imaffine.py

@@ -1074,6 +1083,8 @@ def optimize(self, static, moving, transform, params0,
            self.params0 = self.transform.get_identity_parameters()

        affine_map.set_affine(self.starting_affine)
+        if ret_metric:
+            return affine_map, opt.xopt, opt.fopt


I do not think that it is a good idea to optionally return a different number of results...

Borda · 2019-05-16T07:57:12Z

dipy/io/image.py



 def load_nifti(fname, return_img=False, return_voxsize=False,
               return_coords=False):
    img = nib.load(fname)
    data = img.get_data()
    vox_size = img.header.get_zooms()[:3]
-    
+


why new line?

Borda · 2019-05-16T07:58:01Z

dipy/io/image.py

+        ---------
+        fname : str
+            The file containing the saved affine matrix.
+    """


include return description

Borda · 2019-05-16T08:00:13Z

dipy/io/image.py

+
+
+def save_quality_assur_metric(fname, xopt, fopt):
+    """


you should say that the output is text and back parsable because you add fopt at the end of the file...

Borda · 2019-05-16T08:03:00Z

dipy/viz/regtools.py

@@ -384,6 +389,9 @@ def overlay_slices(L, R, slice_index=None, slice_type=1, ltitle='Left',
    colorImage[..., 0] = ll * (ll > ll[0, 0])
    colorImage[..., 1] = rr * (rr > rr[0, 0])

+    if ret_slice:


I would rather use this function to return slice always and define a new function with a slice image as input and create a figure with an overlay

Borda · 2019-05-16T08:15:37Z

dipy/workflows/tests/test_align.py

+            assert os.path.exists(affine_mat_file)
+            return True
+
+        test_com()


what is the reason for defining all this function when they are used only once

Borda · 2019-05-16T08:20:49Z

dipy/workflows/vis_registration.py

+        """
+        rows, cols = 0, 0
+
+        while True:


compute it directly, no loops are needed
num_slices += 5 - (num_slices % 5) if (num_slices % 5) > 0 else 0

Borda · 2019-05-16T08:21:30Z

dipy/workflows/vis_registration.py

+        for i in range(5, num_slices):
+
+            if num_slices % i == 0:
+                rows = i


compute it directly, no loops are needed

Borda · 2019-05-16T08:23:30Z

dipy/workflows/vis_registration.py

+                renderer.reset_camera()
+                renderer.zoom(1.6)
+                cnt += 1
+                if cnt > num_slices:


how/why this can happen?

Borda · 2019-05-16T08:26:23Z

dipy/workflows/vis_registration.py

+
+        num_slices = 0
+
+        if slice_type == 'sagittal':


I would use a dictionary

slice_dict = { 'sagittal': (x, 0), 'coronal': (y, 1), 'axial': (z, 2) } num_slices, slice_type = slice_dict[slice_type]

arokem · 2019-05-22T02:57:50Z

Is this PR still in progress? Looks like not much work has happened here recently, so I am going to close this. Feel free to reopen if you'd like to pick this up.

skoudoro added the gsoc2018 label Aug 1, 2018

parichit added 10 commits August 7, 2018 16:29

Crated a new branch with clean history for the affine registration wo…

535efaa

…rkflow. 1) Committing to this branch all the code from the affine_registration branch (PR 1581) 2) New PR should be opened for this branch.

Pasted the stable (latest) code from the PR 1581 in the image.py file.

802f3c4

Pasted the stable (latest) code from PR 1581 into the test_align.py f…

df43e44

…ile for test cases.

Pasted the stable code from PR1581 to imaffine.py file.

b6ace1c

Fixed the PEP8 issues.

36717ba

Pasted the stable code from PR 1581 for the command line wrapper into…

468a9a3

… dipy_align_affine.

Changed the function call in the 'RIGID' transform to invoke 'center …

c6a6264

…of mass' transformation rather than 'translation' when the progressive flag is off.

added a new line at the end of file in dipy_align_affine command to f…

095cb7f

…ix the PEP8 issue.

Fixed the PEP8 space complaints in the align.py file.

cd4191d

Updated the parameter documentation in the ImageRegistrationFlow() cl…

d55dfb2

…ass.

parichit force-pushed the visualisation_modules_clean branch from 19c44ce to b37d97d Compare August 7, 2018 20:38

Decreased the decimal precision to 1 decimal places (in checking the …

e5c0979

…value of distance metric) to validate the tests by travis.

parichit force-pushed the visualisation_modules_clean branch from b37d97d to e06854a Compare August 13, 2018 02:27

1) Updated the parameter documentation for optimal parameters and sim…

fdb3105

…ilarity metric in the optimize() function of the imaffine.py file. 2) Changed the parameter documentation in the run() method of the alignpy file for improved view on the command line.

parichit force-pushed the visualisation_modules_clean branch from edcdfec to 742f68b Compare August 13, 2018 18:13

parichit added 12 commits August 13, 2018 17:20

removed the redundant print statement.

328c22f

Pasted the code in image.py file from PR1598.

246ab27

Pasted the clean and stable code in align.py from PR1598.

6d91a5c

Pasted the clean and stable code in test_align.py from PR1598.

bfb6897

Pasted the clean and stable code in dipy_apply_affine wrapper from PR…

f66751b

…1598.

1) Fixed the PEP8 formatting issue in the dipy_align_affine and dipy_…

1e1bd66

…apply_transform. 2) Changed the body of 'rigid()' function in align.py to call the center_of_mass function when the progressive flag is off.

1) Renamed the ApplyTransformFlow to ApplyAffineFlow to distinguish i…

ff38243

…t from the Syn_Registration flow. 2) Made the changes in the command line wrapper to use the changed name for the workflow.

Fixed the PEP8 warning in the test_align.py for the test_apply_transf…

81e07f9

…orm() class.

Changed the name of the function to test_apply_affine_transform() and…

f6feb26

… decreased the decimal precision to 1 decimal place for validating the travis tests.

Fixed the output file creation error due to output directory prependi…

adefc45

…ng in the path of ApplyAffineWorkflow().

Updated the parameter documentation for the ApplyAffineFlow() class.

b764262

Pasted the clean and stable code in vis_registration.py from PR1595.

67e9422

parichit added 9 commits August 13, 2018 17:22

Pasted the clean code for overlay_slices function from PR1595.

f32b7b6

Pasted the stable code in core.py from PR1595.

50231a9

Fixed the PEP8 warning in the vis_registration.py

71c3ccb

Pasted the stable code in dipy_vis_register.py wrapper from PR1595.

db7b84a

Fixed the PEP8 warnings in the dipy_vis_register.py file.

2b0697f

1) Renamed the command line wrapper for the syn_registration.

79f47bd

2) Added a utility funciton in the vis_register.py to check the slice type and raise the error.

Added the test case.

0a98c9a

temp_commit

ee5a449

Fixed the testing of color range values in the numpy array by calling…

73fc149

… the numpy.all() method on the array.

parichit force-pushed the visualisation_modules_clean branch from 742f68b to 73fc149 Compare August 13, 2018 21:22

skoudoro mentioned this pull request Aug 21, 2018

Workflow for visualizing the quality of the registered data with DIPY #1595

Closed

skoudoro added the state: needs code review label Sep 15, 2018

Borda reviewed May 16, 2019

View reviewed changes

arokem closed this May 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleaned PR for Visualization Modules to Assess the quality of Registration Qualitatively. #1606

Cleaned PR for Visualization Modules to Assess the quality of Registration Qualitatively. #1606

parichit commented Jul 31, 2018 •

edited

Loading

pep8speaks commented Jul 31, 2018 •

edited

Loading

codecov-io commented Aug 13, 2018 •

edited

Loading

Borda left a comment

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

Borda May 16, 2019

arokem commented May 22, 2019

Cleaned PR for Visualization Modules to Assess the quality of Registration Qualitatively. #1606

Cleaned PR for Visualization Modules to Assess the quality of Registration Qualitatively. #1606

Conversation

parichit commented Jul 31, 2018 • edited Loading

pep8speaks commented Jul 31, 2018 • edited Loading

Comment last updated on August 13, 2018 at 21:22 Hours UTC

codecov-io commented Aug 13, 2018 • edited Loading

Codecov Report

Borda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arokem commented May 22, 2019

parichit commented Jul 31, 2018 •

edited

Loading

pep8speaks commented Jul 31, 2018 •

edited

Loading

codecov-io commented Aug 13, 2018 •

edited

Loading