Improve Iterator and SubFactory declarations.

* Iterator now cycles by default
* Iterator can be provided with a custom getter
* SubFactory accepts a factory import path as well

Deprecates:
* InfiniteIterator
* CircularSubFactory

Signed-off-by: Raphaël Barrois <raphael.barrois@polytechnique.org>

5 files changed, 224 insertions, 27 deletions

diff --git a/docs/reference.rst b/docs/reference.rst
index 289a9a8..edbd527 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -607,7 +607,9 @@ Circular imports
 
 Some factories may rely on each other in a circular manner.
 This issue can be handled by passing the absolute import path to the target
-:class:`Factory` to the :class:`SubFactory`:
+:class:`Factory` to the :class:`SubFactory`.
+
+.. versionadded:: 1.3.0
 
 .. code-block:: python
 
@@ -633,6 +635,19 @@ Obviously, such circular relationships require careful handling of loops:
     <john (group: MyGroup)>
 
 
+.. class:: CircularSubFactory(module_name, symbol_name, **kwargs)
+
+    .. OHAI_VIM**
+
+    Lazily imports ``module_name.symbol_name`` at the first call.
+
+.. deprecated:: 1.3.0
+    Merged into :class:`SubFactory`; will be removed in 2.0.0.
+
+    Replace ``factory.CircularSubFactory('some.module', 'Symbol', **kwargs)``
+    with ``factory.SubFactory('some.module.Symbol', **kwargs)``
+
+
 SelfAttribute
 """""""""""""
 
@@ -708,14 +723,13 @@ Iterator
 The :class:`Iterator` declaration takes succesive values from the given
 iterable. When it is exhausted, it starts again from zero (unless ``cycle=False``).
 
-.. note:: Versions prior to 1.3.0 declared both :class:`Iterator` (for ``cycle=False``)
-          and :class:`InfiniteIterator` (for ``cycle=True``).
-
-          :class:`InfiniteIterator` is deprecated as of 1.3.0 and will be removed in 2.0.0
-
 The ``cycle`` argument is only useful for advanced cases, where the provided
 iterable has no end (as wishing to cycle it means storing values in memory...).
 
+.. versionadded:: 1.3.0
+    The ``cycle`` argument is available as of v1.3.0; previous versions
+    had a behaviour equivalent to ``cycle=False``.
+
 Each call to the factory will receive the next value from the iterable:
 
 .. code-block:: python
@@ -750,6 +764,8 @@ This is handled by the :attr:`~Iterator.getter` attribute: this is a function
 that accepts as sole parameter a value from the iterable, and returns an
 adequate value.
 
+.. versionadded:: 1.3.0
+
 .. code-block:: python
 
     class UserFactory(factory.Factory):
@@ -759,6 +775,56 @@ adequate value.
         category = factory.Iterator(User.CATEGORY_CHOICES, getter=lambda c: c[0])
 
 
+Decorator
+~~~~~~~~~
+
+.. function:: iterator(func)
+
+
+When generating items of the iterator gets too complex for a simple list comprehension,
+use the :func:`iterator` decorator:
+
+.. warning:: The decorated function takes **no** argument,
+             notably no ``self`` parameter.
+
+.. code-block:: python
+
+    class UserFactory(factory.Factory):
+        FACTORY_FOR = User
+
+        @factory.iterator
+        def name():
+            with open('test/data/names.dat', 'r') as f:
+                for line in f:
+                    yield line
+
+
+InfiniteIterator
+~~~~~~~~~~~~~~~~
+
+.. class:: InfiniteIterator(iterable)
+
+    Equivalent to ``factory.Iterator(iterable)``.
+
+.. deprecated:: 1.3.0
+    Merged into :class:`Iterator`; will be removed in v2.0.0.
+
+    Replace ``factory.InfiniteIterator(iterable)``
+    with ``factory.Iterator(iterable)``.
+
+
+.. function:: infinite_iterator(function)
+
+    Equivalent to ``factory.iterator(func)``.
+
+
+.. deprecated:: 1.3.0
+    Merged into :func:`iterator`; will be removed in v2.0.0.
+
+    Replace ``@factory.infinite_iterator`` with ``@factory.iterator``.
+
+
+
 post-building hooks
 """""""""""""""""""
 
diff --git a/factory/declarations.py b/factory/declarations.py
index c64a0e5..d5b7950 100644
--- a/factory/declarations.py
+++ b/factory/declarations.py
@@ -22,6 +22,7 @@
 
 
 import itertools
+import warnings
 
 from factory import utils
 
@@ -135,14 +136,23 @@ class Iterator(OrderedDeclaration):
 
     Attributes:
         iterator (iterable): the iterator whose value should be used.
+        getter (callable or None): a function to parse returned values
     """
 
-    def __init__(self, iterator):
+    def __init__(self, iterator, cycle=True, getter=None):
         super(Iterator, self).__init__()
-        self.iterator = iter(iterator)
+        self.getter = getter
+
+        if cycle:
+            self.iterator = itertools.cycle(iterator)
+        else:
+            self.iterator = iter(iterator)
 
     def evaluate(self, sequence, obj, containers=()):
-        return next(self.iterator)
+        value = next(self.iterator)
+        if self.getter is None:
+            return value
+        return self.getter(value)
 
 
 class InfiniteIterator(Iterator):
@@ -153,7 +163,11 @@ class InfiniteIterator(Iterator):
     """
 
     def __init__(self, iterator):
-        return super(InfiniteIterator, self).__init__(itertools.cycle(iterator))
+        warnings.warn(
+            "factory.InfiniteIterator is deprecated, and will be removed in the "
+            "future. Please use factory.Iterator instead.",
+            PendingDeprecationWarning, 2)
+        return super(InfiniteIterator, self).__init__(iterator, cycle=True)
 
 
 class Sequence(OrderedDeclaration):
@@ -289,10 +303,24 @@ class SubFactory(ParameteredAttribute):
 
     def __init__(self, factory, **kwargs):
         super(SubFactory, self).__init__(**kwargs)
-        self.factory = factory
+        if isinstance(factory, type):
+            self.factory = factory
+            self.factory_module = self.factory_name = ''
+        else:
+            # Must be a string
+            if not isinstance(factory, basestring) or '.' not in factory:
+                raise ValueError(
+                        "The argument of a SubFactory must be either a class "
+                        "or the fully qualified path to a Factory class; got "
+                        "%r instead." % factory)
+            self.factory = None
+            self.factory_module, self.factory_name = factory.rsplit('.', 1)
 
     def get_factory(self):
         """Retrieve the wrapped factory.Factory subclass."""
+        if self.factory is None:
+            # Must be a module path
+            self.factory = utils.import_object(self.factory_module, self.factory_name)
         return self.factory
 
     def generate(self, create, params):
@@ -314,22 +342,15 @@ class SubFactory(ParameteredAttribute):
 class CircularSubFactory(SubFactory):
     """Use to solve circular dependencies issues."""
     def __init__(self, module_name, factory_name, **kwargs):
-        super(CircularSubFactory, self).__init__(None, **kwargs)
-        self.module_name = module_name
-        self.factory_name = factory_name
+        factory = '%s.%s' % (module_name, factory_name)
+        warnings.warn(
+            "factory.CircularSubFactory is deprecated and will be removed in "
+            "the future. "
+            "Please replace factory.CircularSubFactory('module', 'symbol') "
+            "with factory.SubFactory('module.symbol').",
+            PendingDeprecationWarning, 2)
 
-    def get_factory(self):
-        """Retrieve the factory.Factory subclass.
-
-        Its value is cached in the 'factory' attribute, and retrieved through
-        the factory_getter callable.
-        """
-        if self.factory is None:
-            factory_class = utils.import_object(
-                self.module_name, self.factory_name)
-
-            self.factory = factory_class
-        return self.factory
+        super(CircularSubFactory, self).__init__(factory, **kwargs)
 
 
 class PostGenerationDeclaration(object):
@@ -456,6 +477,10 @@ def iterator(func):
 
 def infinite_iterator(func):
     """Turn a generator function into an infinite iterator attribute."""
+    warnings.warn(
+        "@factory.infinite_iterator is deprecated and will be removed in the "
+        "future. Please use @factory.iterator instead.",
+        PendingDeprecationWarning, 2)
     return InfiniteIterator(func())
 
 def sequence(func):
diff --git a/tests/cyclic/bar.py b/tests/cyclic/bar.py
index a8c9670..fed0602 100644
--- a/tests/cyclic/bar.py
+++ b/tests/cyclic/bar.py
@@ -33,5 +33,5 @@ class BarFactory(factory.Factory):
     FACTORY_FOR = Bar
 
     y = 13
-    foo = factory.CircularSubFactory('cyclic.foo', 'FooFactory')
+    foo = factory.SubFactory('cyclic.foo.FooFactory')
 
diff --git a/tests/test_declarations.py b/tests/test_declarations.py
index 214adc0..ecec244 100644
--- a/tests/test_declarations.py
+++ b/tests/test_declarations.py
@@ -21,10 +21,14 @@
 # THE SOFTWARE.
 
 import datetime
+import itertools
+import warnings
 
 from factory import declarations
 
 from .compat import unittest
+from . import tools
+
 
 class OrderedDeclarationTestCase(unittest.TestCase):
     def test_errors(self):
@@ -88,6 +92,28 @@ class SelfAttributeTestCase(unittest.TestCase):
         self.assertEqual(declarations._UNSPECIFIED, a.default)
 
 
+class IteratorTestCase(unittest.TestCase):
+    def test_cycle(self):
+        it = declarations.Iterator([1, 2])
+        self.assertEqual(1, it.evaluate(0, None))
+        self.assertEqual(2, it.evaluate(1, None))
+        self.assertEqual(1, it.evaluate(2, None))
+        self.assertEqual(2, it.evaluate(3, None))
+
+    def test_no_cycling(self):
+        it = declarations.Iterator([1, 2], cycle=False)
+        self.assertEqual(1, it.evaluate(0, None))
+        self.assertEqual(2, it.evaluate(1, None))
+        self.assertRaises(StopIteration, it.evaluate, 2, None)
+
+    def test_getter(self):
+        it = declarations.Iterator([(1, 2), (1, 3)], getter=lambda p: p[1])
+        self.assertEqual(2, it.evaluate(0, None))
+        self.assertEqual(3, it.evaluate(1, None))
+        self.assertEqual(2, it.evaluate(2, None))
+        self.assertEqual(3, it.evaluate(3, None))
+
+
 class PostGenerationDeclarationTestCase(unittest.TestCase):
     def test_extract_no_prefix(self):
         decl = declarations.PostGenerationDeclaration()
@@ -105,7 +131,51 @@ class PostGenerationDeclarationTestCase(unittest.TestCase):
         self.assertEqual(kwargs, {'baz': 1})
 
 
+class SubFactoryTestCase(unittest.TestCase):
+    def test_lazyness(self):
+        f = declarations.SubFactory('factory.declarations.Sequence', x=3)
+        self.assertEqual(None, f.factory)
+
+        self.assertEqual({'x': 3}, f.defaults)
+
+        factory_class = f.get_factory()
+        self.assertEqual(declarations.Sequence, factory_class)
+
+    def test_cache(self):
+        orig_date = datetime.date
+        f = declarations.SubFactory('datetime.date')
+        self.assertEqual(None, f.factory)
+
+        factory_class = f.get_factory()
+        self.assertEqual(orig_date, factory_class)
+
+        try:
+            # Modify original value
+            datetime.date = None
+            # Repeat import
+            factory_class = f.get_factory()
+            self.assertEqual(orig_date, factory_class)
+
+        finally:
+            # IMPORTANT: restore attribute.
+            datetime.date = orig_date
+
+
 class CircularSubFactoryTestCase(unittest.TestCase):
+
+    def test_circularsubfactory_deprecated(self):
+        with warnings.catch_warnings(record=True) as w:
+            __warningregistry__.clear()
+
+            warnings.simplefilter('always')
+            declarations.CircularSubFactory('datetime', 'date')
+
+            self.assertEqual(1, len(w))
+            self.assertIn('CircularSubFactory', str(w[0].message))
+            self.assertIn('deprecated', str(w[0].message))
+
+
+    @tools.disable_warnings
     def test_lazyness(self):
         f = declarations.CircularSubFactory('factory.declarations', 'Sequence', x=3)
         self.assertEqual(None, f.factory)
@@ -115,6 +185,7 @@ class CircularSubFactoryTestCase(unittest.TestCase):
         factory_class = f.get_factory()
         self.assertEqual(declarations.Sequence, factory_class)
 
+    @tools.disable_warnings
     def test_cache(self):
         orig_date = datetime.date
         f = declarations.CircularSubFactory('datetime', 'date')
@@ -134,5 +205,6 @@ class CircularSubFactoryTestCase(unittest.TestCase):
             # IMPORTANT: restore attribute.
             datetime.date = orig_date
 
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/test_using.py b/tests/test_using.py
index fb0e8b0..fb8c207 100644
--- a/tests/test_using.py
+++ b/tests/test_using.py
@@ -1076,6 +1076,21 @@ class IteratorTestCase(unittest.TestCase):
         for i, obj in enumerate(objs):
             self.assertEqual(i + 10, obj.one)
 
+    def test_infinite_iterator_deprecated(self):
+        with warnings.catch_warnings(record=True) as w:
+            __warningregistry__.clear()
+
+            warnings.simplefilter('always')
+            class TestObjectFactory(factory.Factory):
+                FACTORY_FOR = TestObject
+
+                foo = factory.InfiniteIterator(range(5))
+
+            self.assertEqual(1, len(w))
+            self.assertIn('InfiniteIterator', str(w[0].message))
+            self.assertIn('deprecated', str(w[0].message))
+
+    @disable_warnings
     def test_infinite_iterator(self):
         class TestObjectFactory(factory.Factory):
             FACTORY_FOR = TestObject
@@ -1088,6 +1103,7 @@ class IteratorTestCase(unittest.TestCase):
             self.assertEqual(i % 5, obj.one)
 
     @unittest.skipUnless(is_python2, "Scope bleeding fixed in Python3+")
+    @disable_warnings
     def test_infinite_iterator_list_comprehension_scope_bleeding(self):
         class TestObjectFactory(factory.Factory):
             FACTORY_FOR = TestObject
@@ -1098,6 +1114,7 @@ class IteratorTestCase(unittest.TestCase):
 
         self.assertRaises(TypeError, TestObjectFactory.build)
 
+    @disable_warnings
     def test_infinite_iterator_list_comprehension_protected(self):
         class TestObjectFactory(factory.Factory):
             FACTORY_FOR = TestObject
@@ -1125,6 +1142,23 @@ class IteratorTestCase(unittest.TestCase):
         for i, obj in enumerate(objs):
             self.assertEqual(i + 10, obj.one)
 
+    def test_infinite_iterator_decorator_deprecated(self):
+        with warnings.catch_warnings(record=True) as w:
+            __warningregistry__.clear()
+
+            warnings.simplefilter('always')
+            class TestObjectFactory(factory.Factory):
+                FACTORY_FOR = TestObject
+
+                @factory.infinite_iterator
+                def one():
+                    return range(5)
+
+            self.assertEqual(1, len(w))
+            self.assertIn('infinite_iterator', str(w[0].message))
+            self.assertIn('deprecated', str(w[0].message))
+
+    @disable_warnings
     def test_infinite_iterator_decorator(self):
         class TestObjectFactory(factory.Factory):
             FACTORY_FOR = TestObject