Repository: FactoryBoy/factory_boy
Pull request #661 (2019-11-21 20:19:13)

Improve docs on instance building functions
Add description with example about module level instance building functions.

Solves issue https://github.com/FactoryBoy/factory_boy/issues/572

matmarczak: Thanks for the reply. Sorry but I didn't attach right ticket to my PR. The issue I was trying to solve was https://github.com/FactoryBoy/factory_boy/issues/572. I edited my post to match what I meant.
francoisfreitag: Instead of an example, what do you think of guarding against use of a Factory as the `klass`? Something like:
```diff
diff --git a/factory/helpers.py b/factory/helpers.py
index 01866ec..42214f4 100644
--- a/factory/helpers.py
+++ b/factory/helpers.py
@@ -30,6 +30,9 @@ def make_factory(klass, **kwargs):
     """Create a new, simple factory for the given class."""
     factory_name = '%sFactory' % klass.__name__
 
+    if isinstance(klass, base.FactoryMetaClass):
+        raise TypeError(f"{klass!r} should not be a Factory")
+
     class Meta:
         model = klass
 
```
matmarczak: This solves case when one creates factory with `make_factory` and similar. I can't see case when one want to use Factory as Meta.model, so what do you think about more general approach like:

diff --git a/factory/base.py b/factory/base.py
index da7aa55..d35e1dd 100644
--- a/factory/base.py
+++ b/factory/base.py
@@ -162,8 +162,17 @@ class FactoryOptions(object):
         Custom FactoryOptions classes should override this method
         to update() its return value.
         """
+
+        def _is_factory(meta, value):
+            if isinstance(value, FactoryMetaClass):
+                raise TypeError(
+                    "%s is already a %s"
+                    % (repr(value), Factory.__name__)
+                )
+
         return [
-            OptionDefault('model', None, inherit=True),
+            OptionDefault('model', None, inherit=True,
+                          checker=_is_factory),
             OptionDefault('abstract', False, inherit=False),

I agree, that example in instance building docs section wouldn't be useful.
francoisfreitag: I like that suggestion!
francoisfreitag: Thanks for the patch @matmarczak!

I pushed a commit for last minute cosmetic edits to your branch. The commit message will be used for the squashed commits. Please feel free to suggest rewording or improvements. Otherwise, I’ll squash and merge in the next few days.
francoisfreitag: Add a last commit™ for a last cosmetic edit.

Patch:
diff --git a/docs/reference.rst b/docs/reference.rst
index 1ebd2fc..9f05760 100644
--- a/docs/reference.rst
+++ b/docs/reference.rst
@@ -2018,13 +2018,23 @@ Instance building
 """""""""""""""""
 
 The :mod:`factory` module provides a bunch of shortcuts for creating a factory and
-extracting instances from them:
+extracting instances from them. Helper methods can be used to create factories
+in a dynamic way based on parameters.
+
+Internally, helper methods use :func:`make_factory` to create a new
+:class:`Factory` and perform additional calls on the newly created
+:class:`Factory` according to the method name.
+
+Please note, that all Factories created with this methods inherit from the
+:class:`factory.base.Factory` class. For full support of your ``ORM``, specify
+a base class with the ``FACTORY_CLASS`` parameter as shown in
+:func:`make_factory` examples.
 
 .. function:: build(klass, FACTORY_CLASS=None, **kwargs)
 .. function:: build_batch(klass, size, FACTORY_CLASS=None, **kwargs)
 
     Create a factory for :obj:`klass` using declarations passed in kwargs;
-    return an instance built from that factory,
+    return an instance built from that factory with :data:`BUILD_STRATEGY`,
     or a list of :obj:`size` instances (for :func:`build_batch`).
 
     :param class klass: Class of the instance to build
@@ -2038,7 +2048,7 @@ extracting instances from them:
 .. function:: create_batch(klass, size, FACTORY_CLASS=None, **kwargs)
 
     Create a factory for :obj:`klass` using declarations passed in kwargs;
-    return an instance created from that factory,
+    return an instance created from that factory with :data:`CREATE_STRATEGY`,
     or a list of :obj:`size` instances (for :func:`create_batch`).
 
     :param class klass: Class of the instance to create
@@ -2052,7 +2062,7 @@ extracting instances from them:
 .. function:: stub_batch(klass, size, FACTORY_CLASS=None, **kwargs)
 
     Create a factory for :obj:`klass` using declarations passed in kwargs;
-    return an instance stubbed from that factory,
+    return an instance stubbed from that factory with :data:`STUB_STRATEGY`,
     or a list of :obj:`size` instances (for :func:`stub_batch`).
 
     :param class klass: Class of the instance to stub
diff --git a/factory/base.py b/factory/base.py
index da7aa55..9ed6c68 100644
--- a/factory/base.py
+++ b/factory/base.py
@@ -162,8 +162,16 @@ class FactoryOptions(object):
         Custom FactoryOptions classes should override this method
         to update() its return value.
         """
+
+        def is_model(meta, value):
+            if isinstance(value, FactoryMetaClass):
+                raise TypeError(
+                    "%s is already a %s"
+                    % (repr(value), Factory.__name__)
+                )
+
         return [
-            OptionDefault('model', None, inherit=True),
+            OptionDefault('model', None, inherit=True, checker=is_model),
             OptionDefault('abstract', False, inherit=False),
             OptionDefault('strategy', enums.CREATE_STRATEGY, inherit=True),
             OptionDefault('inline_args', (), inherit=True),


Test patch:
diff --git a/tests/test_base.py b/tests/test_base.py
index 09dd1e6..6752d25 100644
--- a/tests/test_base.py
+++ b/tests/test_base.py
@@ -206,6 +206,16 @@ class OptionsTests(unittest.TestCase):
             OtherFactory._meta.post_declarations.as_dict(),
         )
 
+    def test_factory_as_meta_model_raises_exception(self):
+        class FirstFactory(base.Factory):
+            pass
+
+        class Meta:
+            model = FirstFactory
+
+        with self.assertRaises(TypeError):
+            type("SecondFactory", (base.Factory,), {"Meta": Meta})
+
 
 class DeclarationParsingTests(unittest.TestCase):
     def test_classmethod(self):