Merge pull request #120 from chris-greening/refactor-create-range

Refactor create range

setup.py

@@ -7,7 +7,7 @@
 
 setuptools.setup(
     name="spyrograph",
-    version="0.20.1",
+    version="0.20.2",
     author="Chris Greening",
     author_email="chris@christophergreening.com",
     description="Library for drawing spirographs in Python",

spyrograph/_cycloid.py

@@ -8,6 +8,7 @@
 import time
 
 from spyrograph._trochoid import _Trochoid
+from spyrograph._misc import _get_products_of_inputs, _validate_only_one_iterable
 
 class _Cycloid(_Trochoid):
     # pylint: disable=too-few-public-methods
@@ -172,24 +173,14 @@ def create_range(
             Custom origin to center the shapes at. Default is (0,0)
         """
         # pylint: disable=line-too-long,redefined-argument-from-local,invalid-name,no-member,fixme
-        inputs = collections.Counter([
-            isinstance(R, collections.abc.Iterable),
-            isinstance(r, collections.abc.Iterable)
-        ])
-        if inputs[True] > 1:
-            raise ValueError("More than one input variable was varied. Please only pass one list of varying inputs and try again.")
-        R_arr = cls._set_int_to_list(R)
-        r_arr = cls._set_int_to_list(r)
+        _validate_only_one_iterable(R, r)
+        input_params = _get_products_of_inputs(R, r)
 
-        # TODO: this is fairly ugly, need to come up with better way of handling
-        # this
         shapes = []
-        for R in R_arr:
-            for r in r_arr:
-                shapes.append(cls(
-                    R, r, thetas, theta_start, theta_stop, theta_step,
-                    origin
-                ))
+        for R, r in input_params:
+            shapes.append(cls(
+                R, r, thetas, theta_start, theta_stop, theta_step, origin
+            ))
         return shapes
 
     @classmethod

spyrograph/_misc.py

@@ -0,0 +1,22 @@
+import itertools
+import collections
+from typing import Tuple, List, Union
+from numbers import Number
+
+def _get_products_of_inputs(*args) -> Tuple[Number]:
+        """Return a list of tuples that contains all of the input arguments"""
+        list_of_lists = [_set_int_to_list(el) for el in args]
+        product = list(itertools.product(*list_of_lists))
+        return product
+
+def _validate_only_one_iterable(*args) -> bool:
+    """Return validation check that only one argument passed to create_range is an iterable"""
+    inputs = collections.Counter([isinstance(el, collections.abc.Iterable) for el in args])
+    if inputs[True] > 1:
+        raise ValueError("More than one input variable was varied. Please only pass one list of varying inputs and try again.")
+
+def _set_int_to_list(input_val: Union[Number, List[Number]]) -> List[Number]:
+    """Return list of numbers from given input parameter"""
+    if isinstance(input_val, Number):
+        input_val = [input_val]
+    return input_val

spyrograph/_trochoid.py

@@ -13,6 +13,8 @@
 
 import numpy as np
 
+from spyrograph._misc import _get_products_of_inputs, _validate_only_one_iterable
+
 try:
     import matplotlib.pyplot as plt
 except ImportError:
@@ -490,36 +492,17 @@ def create_range(
         3
         """
         # pylint: disable=line-too-long,redefined-argument-from-local,invalid-name,fixme
-        inputs = collections.Counter([
-            isinstance(R, collections.abc.Iterable),
-            isinstance(r, collections.abc.Iterable),
-            isinstance(d, collections.abc.Iterable)
-        ])
-        if inputs[True] > 1:
-            raise ValueError("More than one input variable was varied. Please only pass one list of varying inputs and try again.")
-        R_arr = cls._set_int_to_list(R)
-        r_arr = cls._set_int_to_list(r)
-        d_arr = cls._set_int_to_list(d)
-
-        # TODO: this is fairly ugly, need to come up with better way of handling
-        # this
+
+        _validate_only_one_iterable(R, r, d)
+        input_params = _get_products_of_inputs(R, r, d)
+
         shapes = []
-        for R in R_arr:
-            for r in r_arr:
-                for d in d_arr:
-                    shapes.append(cls(
-                        R, r, d, thetas, theta_start, theta_stop, theta_step,
-                        origin
-                    ))
+        for R, r, d in input_params:
+            shapes.append(cls(
+                R, r, d, thetas, theta_start, theta_stop, theta_step, origin
+            ))
         return shapes
 
-    @staticmethod
-    def _set_int_to_list(input_val: Union[Number, List[Number]]) -> List[Number]:
-        """Return list of numbers from given input parameter"""
-        if isinstance(input_val, Number):
-            input_val = [input_val]
-        return input_val
-
     def _show_full_path(self, pre_draw_turtle: "turtle.Turtle") -> None:
         """Draw the full path prior to tracing"""
         # pylint: disable=no-member, unused-variable

tests/_roulette.py

@@ -104,15 +104,6 @@ def test_create_range_single_input(self, thetas):
         assert len(shapes) == 1
         assert isinstance(shapes[0], self.class_name)
 
-    def test_set_int_to_list(self):
-        """Test that setting int to list"""
-        num_test = self.class_name._set_int_to_list(1)
-        list_test = self.class_name._set_int_to_list([2])
-        assert isinstance(num_test, list)
-        assert isinstance(list_test, list)
-        assert num_test[0] == 1
-        assert list_test[0] == 2
-
     def test_create_range_multiple_arguments_exception(self, thetas):
         """Test that passing multiple parameters raises an error"""
         with pytest.raises(ValueError):

tests/test_misc.py

@@ -0,0 +1,10 @@
+from spyrograph._misc import _set_int_to_list
+
+def test_set_int_to_list():
+    """Test that setting int to list"""
+    num_test = _set_int_to_list(1)
+    list_test = _set_int_to_list([2])
+    assert isinstance(num_test, list)
+    assert isinstance(list_test, list)
+    assert num_test[0] == 1
+    assert list_test[0] == 2