From c756d2d4927369352c3f50f3a23bb3f8137f24e6 Mon Sep 17 00:00:00 2001
From: Korijn van Golen <korijn@gmail.com>
Date: Mon, 15 Jun 2015 11:49:21 +0200
Subject: [PATCH] Partially fulfilled code review.

---
 skimage/feature/_hog.py           |  4 +-
 skimage/feature/_hoghistogram.pyx | 85 ++++++++++++++++++++++++++-----
 2 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/skimage/feature/_hog.py b/skimage/feature/_hog.py
index e9bfdeb6..7fafc10b 100644
--- a/skimage/feature/_hog.py
+++ b/skimage/feature/_hog.py
@@ -117,8 +117,8 @@ def hog(image, orientations=9, pixels_per_cell=(8, 8),
     # compute orientations integral images
     orientation_histogram = np.zeros((n_cellsy, n_cellsx, orientations))
 
-    _hoghistogram.HogHistograms(gx, gy, cx, cy, sx, sy, n_cellsx, n_cellsy, visualise, orientations, 
-        orientation_histogram)
+    _hoghistogram.HogHistograms(gx, gy, cx, cy, sx, sy, n_cellsx, n_cellsy, 
+        visualise, orientations, orientation_histogram)
 
     # now for each cell, compute the histogram
     hog_image = None
diff --git a/skimage/feature/_hoghistogram.pyx b/skimage/feature/_hoghistogram.pyx
index df38f945..42df311f 100644
--- a/skimage/feature/_hoghistogram.pyx
+++ b/skimage/feature/_hoghistogram.pyx
@@ -1,43 +1,99 @@
-# cython: profile=True
 # cython: cdivision=True
 # cython: boundscheck=False
 # cython: wraparound=False
 
-import cmath, math
-cimport numpy as np
 import numpy as np
-from scipy import pi, arctan2, cos, sin
+cimport numpy as np
 
+# cnp.float64_t[:, :] magnitude
 cdef float CellHog(np.ndarray[np.float64_t, ndim=2] magnitude, 
     np.ndarray[np.float64_t, ndim=2] orientation,
     float ori1, float ori2,
     int cx, int cy, int xi, int yi, int sx, int sy):
+    """CellHog
+
+    Parameters
+    ----------
+    magnitude : ndarray
+        Coordinate to be clipped.
+    orientation : ndarray
+        The lower bound.
+    ori1 : float
+        The higher bound.
+    ori2 : float
+        The higher bound.
+    cx : int
+        The higher bound.
+    cy : int
+        The higher bound.
+    xi : int
+        The higher bound.
+    yi : int
+        The higher bound.
+    sx : int
+        The higher bound.
+    sy : int
+        The higher bound.
+
+    Returns
+    -------
+    total : float
+        The total HOG value.
+    """
     cdef int cx1, cy1
 
     cdef float total = 0.
     for cy1 in range(-cy/2, cy/2):
         for cx1 in range(-cx/2, cx/2):
-            if yi + cy1 < 0: continue
-            if yi + cy1 >= sy: continue
-            if xi + cx1 < 0: continue
-            if xi + cx1 >= sx: continue
-            if orientation[yi + cy1, xi + cx1] >= ori1: continue
-            if orientation[yi + cy1, xi + cx1] < ori2: continue
+            if (yi + cy1 < 0 
+                or yi + cy1 >= sy 
+                or xi + cx1 < 0
+                or xi + cx1 >= sx 
+                or orientation[yi + cy1, xi + cx1] >= ori1
+                or orientation[yi + cy1, xi + cx1] < ori2): continue
 
             total += magnitude[yi + cy1, xi + cx1]
 
     return total
 
-def HogHistograms(np.ndarray[np.float64_t, ndim=2] gx, \
+def HogHistograms(np.ndarray[np.float64_t, ndim=2] gx, 
     np.ndarray[np.float64_t, ndim=2] gy, 
     int cx, int cy, #Pixels per cell
     int sx, int sy, #Image size
     int n_cellsx, int n_cellsy, 
     int visualise, int orientations, 
     np.ndarray[np.float64_t, ndim=3] orientation_histogram):
+    """HogHistograms
 
-    cdef np.ndarray[np.float64_t, ndim=2] magnitude = np.sqrt(gx**2 + gy**2)
-    cdef np.ndarray[np.float64_t, ndim=2] orientation = arctan2(gy, gx) * (180 / pi) % 180
+    Parameters
+    ----------
+    gx : ndarray
+        Coordinate to be clipped.
+    gy : ndarray
+        The lower bound.
+    cx : int
+        The higher bound.
+    cy : int
+        The higher bound.
+    sx : int
+        The higher bound.
+    sy : int
+        The higher bound.
+    n_cellsx : int
+        The higher bound.
+    n_cellsy : int
+        The higher bound.
+    visualise : int
+        The higher bound.
+    orientations : int
+        The higher bound.
+    orientation_histogram : ndarray
+        The histogram to fill.
+    """
+
+    cdef np.ndarray[np.float64_t, ndim=2] magnitude = np.hypot(gx, gy)
+    cdef np.ndarray[np.float64_t, ndim=2] orientation = (
+        np.arctan2(gy, gx) * (180 / np.pi) % 180)
     cdef int i, x, y, o, yi, xi, cy1, cy2, cx1, cx2
     cdef float ori1, ori2
 
@@ -61,7 +117,8 @@ def HogHistograms(np.ndarray[np.float64_t, ndim=2] gx, \
             x = cx / 2
 
             while x < cx2:
-                orientation_histogram[yi, xi, i] = CellHog(magnitude, orientation, ori1, ori2, cx, cy, x, y, sx, sy)
+                orientation_histogram[yi, xi, i] = CellHog(magnitude, 
+                    orientation, ori1, ori2, cx, cy, x, y, sx, sy)
                 xi += 1
                 x += cx