Merge pull request jazzband#49 from treyhunner/flake8

Check code style in tests

Author: treyhunner

simple_history/admin.py

@@ -37,10 +37,12 @@ def get_urls(self):
         admin_site = self.admin_site
         opts = self.model._meta
         info = opts.app_label, opts.module_name,
-        history_urls = patterns("",
+        history_urls = patterns(
+            "",
             url("^([^/]+)/history/([^/]+)/$",
                 admin_site.admin_view(self.history_form_view),
-                name='%s_%s_simple_history' % info),)
+                name='%s_%s_simple_history' % info),
+        )
         return history_urls + urls
 
     def history_view(self, request, object_id, extra_context=None):
@@ -54,7 +56,8 @@ def history_view(self, request, object_id, extra_context=None):
         # If no history was found, see whether this object even exists.
         obj = get_object_or_404(model, pk=unquote(object_id))
         content_type = ContentType.objects.get_for_model(User)
-        admin_user_view = 'admin:%s_%s_change' % (content_type.app_label, content_type.model)
+        admin_user_view = 'admin:%s_%s_change' % (content_type.app_label,
+                                                  content_type.model)
         context = {
             'title': _('Change history: %s') % force_text(obj),
             'action_list': action_list,
@@ -66,56 +69,67 @@ def history_view(self, request, object_id, extra_context=None):
             'admin_user_view': admin_user_view
         }
         context.update(extra_context or {})
-        context_instance = template.RequestContext(request, current_app=self.admin_site.name)
-        return render_to_response(self.object_history_template, context, context_instance=context_instance)
+        context_instance = template.RequestContext(
+            request, current_app=self.admin_site.name)
+        return render_to_response(self.object_history_template, context,
+                                  context_instance=context_instance)
 
     def history_form_view(self, request, object_id, version_id):
         original_model = self.model
         original_opts = original_model._meta
-        history = getattr(self.model, self.model._meta.simple_history_manager_attribute)
+        history = getattr(self.model,
+                          self.model._meta.simple_history_manager_attribute)
         model = history.model
         opts = model._meta
         pk_name = original_opts.pk.attname
-        record = get_object_or_404(model, **{pk_name: object_id, 'history_id': version_id})
+        record = get_object_or_404(model, **{
+            pk_name: object_id,
+            'history_id': version_id,
+        })
         obj = record.instance
         obj._state.adding = False
 
         if not self.has_change_permission(request, obj):
             raise PermissionDenied
 
         formsets = []
-        ModelForm = self.get_form(request, obj)
+        form_class = self.get_form(request, obj)
         if request.method == 'POST':
-            form = ModelForm(request.POST, request.FILES, instance=obj)
+            form = form_class(request.POST, request.FILES, instance=obj)
             if form.is_valid():
                 form_validated = True
                 new_object = self.save_form(request, form, change=True)
             else:
                 form_validated = False
                 new_object = obj
-            prefixes = {}
 
             if form_validated:
                 self.save_model(request, new_object, form, change=True)
                 form.save_m2m()
 
-                change_message = self.construct_change_message(request, form, formsets)
+                change_message = self.construct_change_message(request, form,
+                                                               formsets)
                 self.log_change(request, new_object, change_message)
                 return self.response_change(request, new_object)
 
         else:
-            form = ModelForm(instance=obj)
+            form = form_class(instance=obj)
 
-        adminForm = helpers.AdminForm(form, self.get_fieldsets(request, obj),
-            self.prepopulated_fields, self.get_readonly_fields(request, obj),
-            model_admin=self)
-        media = self.media + adminForm.media
+        admin_form = helpers.AdminForm(
+            form,
+            self.get_fieldsets(request, obj),
+            self.prepopulated_fields,
+            self.get_readonly_fields(request, obj),
+            model_admin=self,
+        )
+        media = self.media + admin_form.media
 
         url_triplet = (self.admin_site.name, original_opts.app_label,
-                            original_opts.module_name)
+                       original_opts.module_name)
+        content_type_id = ContentType.objects.get_for_model(self.model).id
         context = {
             'title': _('Revert %s') % force_text(obj),
-            'adminform': adminForm,
+            'adminform': admin_form,
             'object_id': object_id,
             'original': obj,
             'is_popup': False,
@@ -124,8 +138,10 @@ def history_form_view(self, request, object_id, version_id):
             'app_label': opts.app_label,
             'original_opts': original_opts,
             'changelist_url': reverse('%s:%s_%s_changelist' % url_triplet),
-            'change_url': reverse('%s:%s_%s_change' % url_triplet, args=(obj.pk,)),
-            'history_url': reverse('%s:%s_%s_history' % url_triplet, args=(obj.pk,)),
+            'change_url': reverse('%s:%s_%s_change' % url_triplet,
+                                  args=(obj.pk,)),
+            'history_url': reverse('%s:%s_%s_history' % url_triplet,
+                                   args=(obj.pk,)),
             # Context variables copied from render_change_form
             'add': False,
             'change': True,
@@ -136,17 +152,19 @@ def history_form_view(self, request, object_id, version_id):
             'has_absolute_url': False,
             'form_url': '',
             'opts': opts,
-            'content_type_id': ContentType.objects.get_for_model(self.model).id,
+            'content_type_id': content_type_id,
             'save_as': self.save_as,
             'save_on_top': self.save_on_top,
             'root_path': getattr(self.admin_site, 'root_path', None),
         }
-        context_instance = template.RequestContext(request, current_app=self.admin_site.name)
-        return render_to_response(self.object_history_form_template, context, context_instance)
+        context_instance = template.RequestContext(
+            request,
+            current_app=self.admin_site.name,
+        )
+        return render_to_response(self.object_history_form_template, context,
+                                  context_instance)
 
     def save_model(self, request, obj, form, change):
-        """
-        Add the admin user to a special model attribute for reference after save
-        """
+        """Set special model attribute to user for reference after save"""
         obj._history_user = request.user
         super(SimpleHistoryAdmin, self).save_model(request, obj, form, change)

simple_history/models.py

@@ -21,11 +21,9 @@
     # copy of django function without use of six
     def python_2_unicode_compatible(klass):
         """
-        A decorator that defines __unicode__ and __str__ methods under Python 2.
-        Under Python 3 it does nothing.
+        Decorator defining __unicode__ and __str__ as appropriate for Py2/3
 
-        To support Python 2 and 3 with a single code base, define a __str__ method
-        returning text and apply this decorator to the class.
+        Usage: define __str__ method and apply this decorator to the class.
         """
         if sys.version_info[0] != 3:
             klass.__unicode__ = klass.__str__
@@ -43,14 +41,19 @@ def contribute_to_class(self, cls, name):
         models.signals.class_prepared.connect(self.finalize, sender=cls)
 
         def save_without_historical_record(self, *args, **kwargs):
-            """Caution! Make sure you know what you're doing before you use this method."""
+            """
+            Save model without saving a historical record
+
+            Make sure you know what you're doing before you use this method.
+            """
             self.skip_history_when_saving = True
             try:
                 ret = self.save(*args, **kwargs)
             finally:
                 del self.skip_history_when_saving
             return ret
-        setattr(cls, 'save_without_historical_record', save_without_historical_record)
+        setattr(cls, 'save_without_historical_record',
+                save_without_historical_record)
 
     def finalize(self, sender, **kwargs):
         history_model = self.create_history_model(sender)
@@ -99,100 +102,24 @@ def copy_fields(self, model):
         a dictionary mapping field name to copied field object.
         """
         fields = {}
-
-        def customize_field(field):
-            field.name = field.attname
-            if isinstance(field, models.AutoField):
-                # The historical model gets its own AutoField, so any
-                # existing one must be replaced with an IntegerField.
-                field.__class__ = models.IntegerField
-            elif isinstance(field, models.FileField):
-                # Don't copy file, just path.
-                field.__class__ = models.TextField
-
-            # The historical instance should not change creation/modification timestamps.
-            field.auto_now = False
-            field.auto_now_add = False
-
-            if field.primary_key or field.unique:
-                # Unique fields can no longer be guaranteed unique,
-                # but they should still be indexed for faster lookups.
-                field.primary_key = False
-                field._unique = False
-                field.db_index = True
-                field.serialize = True
-
         for field in model._meta.fields:
             field = copy.copy(field)
-
             if isinstance(field, models.ForeignKey):
-                class CustomKey(type(field)):
-
-                    def get_attname(self):
-                        return self.name
-
-                    def do_related_class(self, other, cls):
-                        # this hooks into contribute_to_class() and this is
-                        # called specifically after the class_prepared signal
-                        to_field = copy.copy(self.rel.to._meta.pk)
-                        field = self
-                        if isinstance(to_field, models.AutoField):
-                            field.__class__ = models.IntegerField
-                        else:
-                            field.__class__ = to_field.__class__
-                        excluded_prefixes = ("_","__")
-                        excluded_attributes = (
-                            "rel",
-                            "creation_counter",
-                            "validators",
-                            "error_messages",
-                            "attname",
-                            "column",
-                            "help_text",
-                            "name",
-                            "model",
-                            "unique_for_year",
-                            "unique_for_date",
-                            "unique_for_month",
-                            "db_tablespace",
-                            "db_index",
-                            "db_column",
-                            "default",
-                            "auto_created",
-                            "null",
-                            "blank",
-                        )
-                        for key, val in to_field.__dict__.items():
-                            if (isinstance(key, basestring)
-                                    and not key.startswith(excluded_prefixes)
-                                    and not key in excluded_attributes):
-                                setattr(field, key, val)
-
-                        customize_field(field)
-                        field.rel = None
-
-                    def contribute_to_class(self, cls, name):
-                        # HACK: remove annoying descriptor (don't super())
-                        RelatedField.contribute_to_class(self, cls, name)
-
                 # Don't allow reverse relations.
                 # ForeignKey knows best what datatype to use for the column
                 # we'll used that as soon as it's finalized by copying rel.to
-                field.__class__ = CustomKey
+                field.__class__ = get_custom_fk_class(type(field))
                 field.rel.related_name = '+'
                 field.null = True
                 field.blank = True
-
-            customize_field(field)
+            transform_field(field)
             fields[field.name] = field
-
         return fields
 
     def get_extra_fields(self, model, fields):
-        """
-        Returns a dictionary of fields that will be added to the historical
-        record model, in addition to the ones returned by copy_fields below.
-        """
+        """Return dict of extra fields added to the historical record model"""
+
+        user_model = getattr(settings, 'AUTH_USER_MODEL', 'auth.User')
 
         @models.permalink
         def revert_url(self):
@@ -207,8 +134,7 @@ def get_instance(self):
         return {
             'history_id': models.AutoField(primary_key=True),
             'history_date': models.DateTimeField(auto_now_add=True),
-            'history_user': models.ForeignKey(
-                getattr(settings, 'AUTH_USER_MODEL', 'auth.User'), null=True),
+            'history_user': models.ForeignKey(user_model, null=True),
             'history_type': models.CharField(max_length=1, choices=(
                 ('+', 'Created'),
                 ('~', 'Changed'),
@@ -248,10 +174,87 @@ def create_historical_record(self, instance, type):
         manager.create(history_type=type, history_user=history_user, **attrs)
 
 
+def get_custom_fk_class(parent_type):
+    class CustomForeignKey(parent_type):
+
+        def get_attname(self):
+            return self.name
+
+        def do_related_class(self, other, cls):
+            # this hooks into contribute_to_class() and this is
+            # called specifically after the class_prepared signal
+            to_field = copy.copy(self.rel.to._meta.pk)
+            field = self
+            if isinstance(to_field, models.AutoField):
+                field.__class__ = models.IntegerField
+            else:
+                field.__class__ = to_field.__class__
+                excluded_prefixes = ("_", "__")
+                excluded_attributes = (
+                    "rel",
+                    "creation_counter",
+                    "validators",
+                    "error_messages",
+                    "attname",
+                    "column",
+                    "help_text",
+                    "name",
+                    "model",
+                    "unique_for_year",
+                    "unique_for_date",
+                    "unique_for_month",
+                    "db_tablespace",
+                    "db_index",
+                    "db_column",
+                    "default",
+                    "auto_created",
+                    "null",
+                    "blank",
+                )
+                for key, val in to_field.__dict__.items():
+                    if (isinstance(key, basestring)
+                            and not key.startswith(excluded_prefixes)
+                            and not key in excluded_attributes):
+                        setattr(field, key, val)
+            transform_field(field)
+            field.rel = None
+
+        def contribute_to_class(self, cls, name):
+            # HACK: remove annoying descriptor (don't super())
+            RelatedField.contribute_to_class(self, cls, name)
+
+    return CustomForeignKey
+
+
+def transform_field(field):
+    """Customize field appropriately for use in historical model"""
+    field.name = field.attname
+    if isinstance(field, models.AutoField):
+        # The historical model gets its own AutoField, so any
+        # existing one must be replaced with an IntegerField.
+        field.__class__ = models.IntegerField
+    elif isinstance(field, models.FileField):
+        # Don't copy file, just path.
+        field.__class__ = models.TextField
+
+    # Historical instance shouldn't change create/update timestamps
+    field.auto_now = False
+    field.auto_now_add = False
+
+    if field.primary_key or field.unique:
+        # Unique fields can no longer be guaranteed unique,
+        # but they should still be indexed for faster lookups.
+        field.primary_key = False
+        field._unique = False
+        field.db_index = True
+        field.serialize = True
+
+
 class HistoricalObjectDescriptor(object):
     def __init__(self, model):
         self.model = model
 
     def __get__(self, instance, owner):
-        values = (getattr(instance, f.attname) for f in self.model._meta.fields)
+        values = (getattr(instance, f.attname)
+                  for f in self.model._meta.fields)
         return self.model(*values)

simple_history/tests/custom_user/models.py

@@ -8,4 +8,4 @@
 class CustomUser(AbstractUser):
     pass
 
-admin.site.register(CustomUser, UserAdmin)
+admin.site.register(CustomUser, UserAdmin)