From c4a7324bc2f6dcf52237919ff8e47b65419def89 Mon Sep 17 00:00:00 2001 From: Ana Paula Gomes Date: Fri, 25 Oct 2019 18:21:18 +0200 Subject: [PATCH 1/5] Add pydocstyle as pre-commit hook --- .pre-commit-config.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e02952dd..cf13fa55 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,3 +11,7 @@ repos: rev: 3.7.6 hooks: - id: flake8 +- repo: https://gitlab.com/pycqa/pydocstyle + rev: 4.0.1 + hooks: + - id: pydocstyle From 26a4a4800f599cd32b54c740152533206f5c1623 Mon Sep 17 00:00:00 2001 From: Ana Paula Gomes Date: Fri, 25 Oct 2019 18:26:21 +0200 Subject: [PATCH 2/5] Fix a few comments --- model_bakery/__init__.py | 1 + model_bakery/baker.py | 80 ++++++++++++++++++-------------------- model_bakery/generators.py | 4 +- model_bakery/random_gen.py | 38 ++++++++++-------- model_bakery/recipe.py | 13 +++---- model_bakery/timezone.py | 4 +- model_bakery/utils.py | 28 +++++-------- tests/generic/models.py | 5 +-- tests/test_baker.py | 26 ++++++------- tests/test_utils.py | 26 ++++--------- 10 files changed, 102 insertions(+), 123 deletions(-) diff --git a/model_bakery/__init__.py b/model_bakery/__init__.py index d5cffe3e..863a0895 100644 --- a/model_bakery/__init__.py +++ b/model_bakery/__init__.py @@ -1,3 +1,4 @@ +"""Model Bakery module configuration.""" __version__ = '1.0.2' __title__ = 'model_bakery' __author__ = 'Vanderson Mota' diff --git a/model_bakery/baker.py b/model_bakery/baker.py index faffa706..8a3eb348 100644 --- a/model_bakery/baker.py +++ b/model_bakery/baker.py @@ -18,7 +18,7 @@ ModelNotFound, AmbiguousModelName, InvalidQuantityException, RecipeIteratorEmpty, CustomBakerNotFound, InvalidCustomBaker ) -from .utils import import_from_str, import_if_str +from .utils import import_from_str recipes = None @@ -35,10 +35,10 @@ def _valid_quantity(quantity): def make(_model, _quantity=None, make_m2m=False, _save_kwargs=None, _refresh_after_create=False, _create_files=False, **attrs): - """ - Creates a persisted instance from a given model its associated models. - It fill the fields with random values or you can specify - which fields you want to define its values by yourself. + """Create a persisted instance from a given model its associated models. + + It fill the fields with random values or you can specify which + fields you want to define its values by yourself. """ _save_kwargs = _save_kwargs or {} baker = Baker.create(_model, make_m2m=make_m2m, create_files=_create_files) @@ -62,11 +62,10 @@ def make(_model, _quantity=None, make_m2m=False, _save_kwargs=None, _refresh_aft def prepare(_model, _quantity=None, _save_related=False, **attrs): - """ - Creates BUT DOESN'T persist an instance from a given model its - associated models. - It fill the fields with random values or you can specify - which fields you want to define its values by yourself. + """Create but do not persist an instance from a given model. + + It fill the fields with random values or you can specify which + fields you want to define its values by yourself. """ baker = Baker.create(_model) if _valid_quantity(_quantity): @@ -96,18 +95,20 @@ def prepare_recipe(baker_recipe_name, _quantity=None, _save_related=False, **new class ModelFinder(object): - """ - Encapsulates all the logic for finding a model to Baker. - """ + """Encapsulates all the logic for finding a model to Baker.""" + _unique_models = None _ambiguous_models = None def get_model(self, name): - """ - Get a model. + """Get a model. + + Args: + name (str): A name on the form 'applabel.modelname' or 'modelname' + + Returns: + object: a model class - :param name String on the form 'applabel.modelname' or 'modelname'. - :return a model class. """ try: if '.' in name: @@ -124,11 +125,10 @@ def get_model(self, name): return model def get_model_by_name(self, name): - """ - Get a model by name. + """Get a model by name. - If a model with that name exists in more than one app, - raises AmbiguousModelName. + If a model with that name exists in more than one app, raises + AmbiguousModelName. """ name = name.lower() @@ -142,9 +142,7 @@ def get_model_by_name(self, name): return self._unique_models.get(name) def _populate(self): - """ - Cache models for faster self._get_model. - """ + """Cache models for faster self._get_model.""" unique_models = {} ambiguous_models = [] @@ -172,9 +170,12 @@ def is_iterator(value): def _custom_baker_class(): - """ - Returns custom baker class specified by BAKER_CUSTOM_CLASS in the django - settings, or None if no custom class is defined + """Return the specified custom baker class. + + Returns: + object: The custom class is specified by BAKER_CUSTOM_CLASS in Django's + settings, or None if no custom class is defined. + """ custom_class_string = getattr(settings, 'BAKER_CUSTOM_CLASS', None) if custom_class_string is None: @@ -204,9 +205,7 @@ class Baker(object): @classmethod def create(cls, _model, make_m2m=False, create_files=False): - """ - Factory which creates the baker class defined by the BAKER_CUSTOM_CLASS setting - """ + """Create the baker class defined by the `BAKER_CUSTOM_CLASS` setting.""" baker_class = _custom_baker_class() or cls return baker_class(_model, make_m2m, create_files) @@ -230,8 +229,8 @@ def init_type_mapping(self): self.type_mapping = generators.get_type_mapping() generators_from_settings = getattr(settings, 'BAKER_CUSTOM_FIELDS_GEN', {}) for k, v in generators_from_settings.items(): - field_class = import_if_str(k) - generator = import_if_str(v) + field_class = import_from_str(k) + generator = import_from_str(v) self.type_mapping[field_class] = generator def make( @@ -241,8 +240,7 @@ def make( _from_manager=None, **attrs ): - """Creates and persists an instance of the model associated - with Baker instance.""" + """Create and persist an instance of the model associated with Baker instance.""" params = { 'commit': True, 'commit_related': True, @@ -254,8 +252,7 @@ def make( return self._make(**params) def prepare(self, _save_related=False, **attrs): - """Creates, but does not persist, an instance of the model - associated with Baker instance.""" + """Create, but do not persist, an instance of the associated model.""" return self._make(commit=False, commit_related=_save_related, **attrs) def get_fields(self): @@ -444,8 +441,7 @@ def _remote_field(self, field): return field.remote_field def generate_value(self, field, commit=True): - """ - Calls the generator associated with a field passing all required args. + """Call the associated generator with a field passing all required args. Generator Resolution Precedence Order: -- attr_mapping - mapping per attribute name @@ -486,10 +482,10 @@ def generate_value(self, field, commit=True): def get_required_values(generator, field): - """ - Gets required values for a generator from the field. - If required value is a function, calls it with field as argument. - If required value is a string, simply fetch the value from the field + """Get required values for a generator from the field. + + If required value is a function, calls it with field as argument. If + required value is a string, simply fetch the value from the field and return. """ # FIXME: avoid abbreviations diff --git a/model_bakery/generators.py b/model_bakery/generators.py index 70e4b8cf..bc845a93 100644 --- a/model_bakery/generators.py +++ b/model_bakery/generators.py @@ -9,7 +9,7 @@ from . import random_gen from .gis import default_gis_mapping -from .utils import import_if_str +from .utils import import_from_str try: from django.contrib.postgres.fields import ArrayField @@ -98,7 +98,7 @@ def get_type_mapping(): def add(field, func): - user_mapping[import_if_str(field)] = import_if_str(func) + user_mapping[import_from_str(field)] = import_from_str(func) def get(field): diff --git a/model_bakery/random_gen.py b/model_bakery/random_gen.py index ab0bba39..cbfe8cdb 100644 --- a/model_bakery/random_gen.py +++ b/model_bakery/random_gen.py @@ -1,12 +1,12 @@ -""" -Generators are callables that return a value used to populate a field. - -If this callable has a `required` attribute (a list, mostly), for each item in -the list, if the item is a string, the field attribute with the same name will -be fetched from the field and used as argument for the generator. If it is a -callable (which will receive `field` as first argument), it should return a -list in the format (key, value) where key is the argument name for generator -and value is the value for that argument. +"""Generators are callables that return a value used to populate a field. + +If this callable has a `required` attribute (a list, mostly), for each +item in the list, if the item is a string, the field attribute with the +same name will be fetched from the field and used as argument for the +generator. If it is a callable (which will receive `field` as first +argument), it should return a list in the format (key, value) where key +is the argument name for generator and value is the value for that +argument. """ import string @@ -43,14 +43,18 @@ def gen_image_field(): return get_content_file(f.read(), name=name) -def gen_from_list(L): - '''Makes sure all values of the field are generated from the list L - Usage: - from baker import Baker - class ExperientBaker(Baker): - attr_mapping = {'some_field':gen_from_list([A, B, C])} - ''' - return lambda: choice(list(L)) +def gen_from_list(a_list): + """Make sure all values of the field are generated from a list. + + Examples: + Here how to use it. + + >>> from baker import Baker + >>> class ExperienceBaker(Baker): + >>> attr_mapping = {'some_field': gen_from_list(['A', 'B', 'C'])} + + """ + return lambda: choice(list(a_list)) # -- DEFAULT GENERATORS -- diff --git a/model_bakery/recipe.py b/model_bakery/recipe.py index f8da39e2..45a212bc 100644 --- a/model_bakery/recipe.py +++ b/model_bakery/recipe.py @@ -84,14 +84,15 @@ def __init__(self, recipe): def foreign_key(recipe): - """ - Returns the callable, so that the associated _model - will not be created during the recipe definition. + """Return a `RecipeForeignKey`. + + Return the callable, so that the associated `_model` will not be created + during the recipe definition. """ return RecipeForeignKey(recipe) -class related(object): +class related(object): # FIXME def __init__(self, *args): self.related = [] for recipe in args: @@ -109,7 +110,5 @@ def __init__(self, *args): raise TypeError('Not a recipe') def make(self): - """ - Persists objects to m2m relation - """ + """Persist objects to m2m relation.""" return [m.make() for m in self.related] diff --git a/model_bakery/timezone.py b/model_bakery/timezone.py index 1c1d0ea6..811067a1 100644 --- a/model_bakery/timezone.py +++ b/model_bakery/timezone.py @@ -1,5 +1,5 @@ -""" -Add support for Django 1.4+ safe datetimes. +"""Add support for Django 1.4+ safe datetimes. + https://docs.djangoproject.com/en/1.4/topics/i18n/timezones/ """ # TODO: the whole file seems to be not needed anymore, since Django has this tooling built-in diff --git a/model_bakery/utils.py b/model_bakery/utils.py index 0515204b..cfa64402 100644 --- a/model_bakery/utils.py +++ b/model_bakery/utils.py @@ -5,28 +5,18 @@ from .timezone import tz_aware -def import_if_str(import_string_or_obj): - """ - Import and return an object defined as import string in the form of - - path.to.module.object_name - - or just return the object if it isn't a string. - """ - if isinstance(import_string_or_obj, str): - return import_from_str(import_string_or_obj) - return import_string_or_obj - - def import_from_str(import_string): - """ - Import and return an object defined as import string in the form of + """Import an object defined as import if it is an string. - path.to.module.object_name + If `import_string_or_obj` follows the format `path.to.module.object_name`, + this method imports it; else it just return the object. """ - path, field_name = import_string.rsplit('.', 1) - module = importlib.import_module(path) - return getattr(module, field_name) + if isinstance(import_string, str): + path, field_name = import_string.rsplit('.', 1) + module = importlib.import_module(path) + return getattr(module, field_name) + else: + return import_string def seq(value, increment_by=1): diff --git a/tests/generic/models.py b/tests/generic/models.py index ea622bbb..a4277a62 100755 --- a/tests/generic/models.py +++ b/tests/generic/models.py @@ -306,12 +306,11 @@ class Movie(models.Model): class MovieManager(models.Manager): def get_queryset(self): - '''Annotate queryset with an alias field 'name'. + """Annotate queryset with an alias field 'name'. We want to test whether this annotation has been run after calling baker.make(). - - ''' + """ return ( super(MovieManager, self).get_queryset() .annotate(name=models.F('title')) diff --git a/tests/test_baker.py b/tests/test_baker.py index 588687f0..afaa2f02 100644 --- a/tests/test_baker.py +++ b/tests/test_baker.py @@ -179,18 +179,18 @@ def test_dependent_models_with_ForeignKey(self): assert isinstance(dog.owner, models.Person) def test_foreign_key_on_parent_should_create_one_object(self): - ''' + """ Foreign key on parent gets created twice. Once for parent object and another time for child object - ''' + """ person_count = models.Person.objects.count() baker.make(models.GuardDog) assert models.Person.objects.count() == person_count + 1 def test_foreign_key_on_parent_is_not_created(self): - ''' + """ Foreign key on parent doesn't get created using owner - ''' + """ owner = baker.make(models.Person) person_count = models.Person.objects.count() dog = baker.make(models.GuardDog, owner=owner) @@ -198,9 +198,9 @@ def test_foreign_key_on_parent_is_not_created(self): assert dog.owner == owner def test_foreign_key_on_parent_id_is_not_created(self): - ''' + """ Foreign key on parent doesn't get created using owner_id - ''' + """ owner = baker.make(models.Person) person_count = models.Person.objects.count() dog = baker.make(models.GuardDog, owner_id=owner.id) @@ -208,20 +208,20 @@ def test_foreign_key_on_parent_id_is_not_created(self): assert models.GuardDog.objects.get(pk=dog.pk).owner == owner def test_auto_now_add_on_parent_should_work(self): - ''' + """ Foreign key on parent gets created twice. Once for parent object and another time for child object - ''' + """ person_count = models.Person.objects.count() dog = baker.make(models.GuardDog) assert models.Person.objects.count() == person_count + 1 assert dog.created def test_attrs_on_related_model_through_parent(self): - ''' + """ Foreign key on parent gets created twice. Once for parent object and another time for child object - ''' + """ baker.make(models.GuardDog, owner__name='john') for person in models.Person.objects.all(): assert person.name == 'john' @@ -286,7 +286,7 @@ def test_create_many_to_many_with_set_default_quantity(self): def test_create_many_to_many_with_through_option(self): """ - This does not works + This does not works # FIXME """ # School student's attr is a m2m relationship with a model through school = baker.make(models.School, make_m2m=True) @@ -618,7 +618,7 @@ def test_do_not_refresh_from_db_by_default(self): class TestBakerMakeCanFetchInstanceFromDefaultManager(): def test_annotation_within_manager_get_queryset_are_run_on_make(self): - '''Test that a custom model Manager can be used within make(). + """Test that a custom model Manager can be used within make(). Passing _from_manager='objects' will force baker.make() to return an instance that has been going through that given @@ -626,7 +626,7 @@ def test_annotation_within_manager_get_queryset_are_run_on_make(self): code, like default annotations. As such the instance will have the same fields as one created in the application. - ''' + """ movie = baker.make(models.MovieWithAnnotation) with pytest.raises(AttributeError): movie.name diff --git a/tests/test_utils.py b/tests/test_utils.py index 5cc2373c..62538725 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,26 +1,16 @@ import pytest -from model_bakery.utils import import_if_str, import_from_str +from model_bakery.utils import import_from_str from tests.generic.models import User -class TestUtils: - def test_import_from_str(self): - with pytest.raises(AttributeError): - import_from_str('tests.generic.UndefinedObject') +def test_import_from_str(): + with pytest.raises(AttributeError): + import_from_str('tests.generic.UndefinedObject') - with pytest.raises(ImportError): - import_from_str('tests.generic.undefined_path.User') + with pytest.raises(ImportError): + import_from_str('tests.generic.undefined_path.User') - assert import_from_str('tests.generic.models.User') == User - - def test_import_if_str(self): - with pytest.raises(AttributeError): - import_if_str('tests.generic.UndefinedObject') - - with pytest.raises(ImportError): - import_if_str('tests.generic.undefined_path.User') - - assert import_if_str('tests.generic.models.User') == User - assert import_if_str(User) == User + assert import_from_str('tests.generic.models.User') == User + assert import_from_str(User) == User From c92908989daeb319df0c13f04865c129a404f9cf Mon Sep 17 00:00:00 2001 From: Ana Paula Gomes Date: Fri, 25 Oct 2019 18:26:50 +0200 Subject: [PATCH 3/5] Fix unrelated stuff from flake8 --- tests/test_baker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_baker.py b/tests/test_baker.py index afaa2f02..951af9ed 100644 --- a/tests/test_baker.py +++ b/tests/test_baker.py @@ -322,14 +322,14 @@ def test_m2m_changed(*args, **kwargs): def test_simple_creating_person_with_parameters(self): kid = baker.make(models.Person, happy=True, age=10, name='Mike') assert kid.age == 10 - assert kid.happy == True + assert kid.happy is True assert kid.name == 'Mike' def test_creating_person_from_factory_using_paramters(self): person_baker_ = baker.Baker(models.Person) person = person_baker_.make(happy=False, age=20, gender='M', name='John') assert person.age == 20 - assert person.happy == False + assert person.happy is False assert person.name == 'John' assert person.gender == 'M' @@ -480,7 +480,7 @@ def test_fill_field_optional(self): def test_fill_wrong_field(self): with pytest.raises(AttributeError) as exc_info: - baker.make(models.DummyBlankFieldsModel,_fill_optional=['blank_char_field', 'wrong']) + baker.make(models.DummyBlankFieldsModel, _fill_optional=['blank_char_field', 'wrong']) msg = "_fill_optional field(s) ['wrong'] are not related to model DummyBlankFieldsModel" assert msg in str(exc_info.value) From 62c0b4b5b9bac9c3bf24adac99299869bd638ed2 Mon Sep 17 00:00:00 2001 From: Ana Paula Gomes Date: Sun, 17 Nov 2019 11:57:30 +0100 Subject: [PATCH 4/5] Remove non existent method from docstring --- model_bakery/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model_bakery/utils.py b/model_bakery/utils.py index cfa64402..986d897c 100644 --- a/model_bakery/utils.py +++ b/model_bakery/utils.py @@ -8,7 +8,7 @@ def import_from_str(import_string): """Import an object defined as import if it is an string. - If `import_string_or_obj` follows the format `path.to.module.object_name`, + If `import_string` follows the format `path.to.module.object_name`, this method imports it; else it just return the object. """ if isinstance(import_string, str): From 95d13e4cbfbba283754f93544c01f8a44916f488 Mon Sep 17 00:00:00 2001 From: Ana Paula Gomes Date: Sun, 17 Nov 2019 11:58:28 +0100 Subject: [PATCH 5/5] Remove duplicated comments and improve docstring of a few tests --- tests/test_baker.py | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/tests/test_baker.py b/tests/test_baker.py index 951af9ed..ae972c30 100644 --- a/tests/test_baker.py +++ b/tests/test_baker.py @@ -179,18 +179,12 @@ def test_dependent_models_with_ForeignKey(self): assert isinstance(dog.owner, models.Person) def test_foreign_key_on_parent_should_create_one_object(self): - """ - Foreign key on parent gets created twice. Once for - parent object and another time for child object - """ person_count = models.Person.objects.count() baker.make(models.GuardDog) assert models.Person.objects.count() == person_count + 1 def test_foreign_key_on_parent_is_not_created(self): - """ - Foreign key on parent doesn't get created using owner - """ + """Foreign key on parent doesn't get created using owner.""" owner = baker.make(models.Person) person_count = models.Person.objects.count() dog = baker.make(models.GuardDog, owner=owner) @@ -198,9 +192,7 @@ def test_foreign_key_on_parent_is_not_created(self): assert dog.owner == owner def test_foreign_key_on_parent_id_is_not_created(self): - """ - Foreign key on parent doesn't get created using owner_id - """ + """Foreign key on parent doesn't get created using owner_id.""" owner = baker.make(models.Person) person_count = models.Person.objects.count() dog = baker.make(models.GuardDog, owner_id=owner.id) @@ -208,20 +200,12 @@ def test_foreign_key_on_parent_id_is_not_created(self): assert models.GuardDog.objects.get(pk=dog.pk).owner == owner def test_auto_now_add_on_parent_should_work(self): - """ - Foreign key on parent gets created twice. Once for - parent object and another time for child object - """ person_count = models.Person.objects.count() dog = baker.make(models.GuardDog) assert models.Person.objects.count() == person_count + 1 assert dog.created def test_attrs_on_related_model_through_parent(self): - """ - Foreign key on parent gets created twice. Once for - parent object and another time for child object - """ baker.make(models.GuardDog, owner__name='john') for person in models.Person.objects.all(): assert person.name == 'john' @@ -285,9 +269,6 @@ def test_create_many_to_many_with_set_default_quantity(self): assert store.customers.count() == baker.MAX_MANY_QUANTITY def test_create_many_to_many_with_through_option(self): - """ - This does not works # FIXME - """ # School student's attr is a m2m relationship with a model through school = baker.make(models.School, make_m2m=True) assert models.School.objects.count() == 1 @@ -618,11 +599,11 @@ def test_do_not_refresh_from_db_by_default(self): class TestBakerMakeCanFetchInstanceFromDefaultManager(): def test_annotation_within_manager_get_queryset_are_run_on_make(self): - """Test that a custom model Manager can be used within make(). + """A custom model Manager can be used within make(). - Passing _from_manager='objects' will force baker.make() to - return an instance that has been going through that given - Manager, thus calling its get_queryset() method and associated + Passing ``_from_manager='objects'`` will force ``baker.make()`` + to return an instance that has been going through a given + Manager, thus calling its ``get_queryset()`` method and associated code, like default annotations. As such the instance will have the same fields as one created in the application.