PyLint suggested cleanups

We had a bunch of extra imports, non-conventional variable names, spacing
issues, etc. that were relatively low-hanging fruit to clean up. Fix them
and make the code a bit cleaner in the process.

Signed-off-by: Dan McGee <dan@archlinux.org>

9 files changed, 75 insertions, 67 deletions

diff --git a/devel/management/commands/reporead.py b/devel/management/commands/reporead.py
index 598c0194..ee0c50fb 100644
--- a/devel/management/commands/reporead.py
+++ b/devel/management/commands/reporead.py
@@ -22,11 +22,9 @@ REPOVARS = ['arch', 'backup', 'base', 'builddate', 'conflicts', 'csize',
 
 
 from django.core.management.base import BaseCommand, CommandError
-from django.conf import settings
 from django.contrib.auth.models import User
-from django.db import models, transaction
+from django.db import transaction
 from django.db.models import Q
-from django.core import management
 
 import os
 import re
@@ -39,7 +37,7 @@ from optparse import make_option
 from cStringIO import StringIO
 from logging import ERROR, WARNING, INFO, DEBUG
 
-from main.models import Arch, Package, Repo, UserProfile
+from main.models import Arch, Package, Repo
 
 class SomethingFishyException(Exception):
     '''Raised when the database looks like its going to wipe out a bunch of
@@ -63,15 +61,15 @@ class Command(BaseCommand):
     help = "Runs a package repository import for the given arch and file."
     args = "<arch> <filename>"
 
-    def handle(self, arch=None, file=None, **options):
+    def handle(self, arch=None, filename=None, **options):
         if not arch:
             raise CommandError('Architecture is required.')
         if not validate_arch(arch):
             raise CommandError('Specified architecture %s is not currently known.' % arch)
-        if not file:
+        if not filename:
             raise CommandError('Package database file is required.')
-        file = os.path.normpath(file)
-        if not os.path.exists(file) or not os.path.isfile(file):
+        filename = os.path.normpath(filename)
+        if not os.path.exists(filename) or not os.path.isfile(filename):
             raise CommandError('Specified package database file does not exist.')
 
         v = int(options.get('verbosity', 0))
@@ -82,17 +80,17 @@ class Command(BaseCommand):
         elif v == 2:
             logger.level = DEBUG
 
-        import signal,traceback
+        import signal, traceback
         signal.signal(signal.SIGQUIT,
                 lambda sig, stack: traceback.print_stack(stack))
 
-        return read_repo(arch, file, options)
+        return read_repo(arch, filename, options)
 
 
 class Pkg(object):
     """An interim 'container' object for holding Arch package data."""
 
-    def __init__(self, val):
+    def __init__(self, val, repo):
         selfdict = {}
         squash = ['arch', 'builddate', 'csize', 'desc', 'filename',
                   'installdate', 'isize', 'license', 'md5sum',
@@ -106,7 +104,7 @@ class Pkg(object):
         for x in val.keys():
             if x in squash:
                 if val[x] == None or len(val[x]) == 0:
-                    logger.warning("Package %s has no %s" % (selfdict['name'],x))
+                    logger.warning("Package %s has no %s" % (selfdict['name'], x))
                     selfdict[x] = None
                 else:
                     selfdict[x] = ', '.join(val[x])
@@ -127,8 +125,9 @@ class Pkg(object):
             else:
                 selfdict[x] = val[x]
         self.__dict__ = selfdict
+        self.repo = repo
 
-    def __getattr__(self,name):
+    def __getattr__(self, name):
         if name == 'force':
             return False
         else:
@@ -196,11 +195,13 @@ def populate_pkg(dbpkg, repopkg, force=False, timestamp=None):
     dbpkg.installed_size = int(repopkg.isize)
     try:
         dbpkg.build_date = datetime.utcfromtimestamp(int(repopkg.builddate))
-    except:
+    except ValueError:
         try:
-            dbpkg.build_date = datetime.strptime(repopkg.builddate, '%a %b %d %H:%M:%S %Y')
-        except:
-            pass
+            dbpkg.build_date = datetime.strptime(repopkg.builddate,
+                    '%a %b %d %H:%M:%S %Y')
+        except ValueError:
+            logger.warning('Package %s had unparsable build date %s' % \
+                    (repopkg.name, repopkg.builddate))
     dbpkg.packager_str = repopkg.packager
     # attempt to find the corresponding django user for this string
     dbpkg.packager = find_user(repopkg.packager)
@@ -217,12 +218,12 @@ def populate_pkg(dbpkg, repopkg, force=False, timestamp=None):
         for y in repopkg.depends:
             # make sure we aren't adding self depends..
             # yes *sigh* i have seen them in pkgbuilds
-            dpname,dpvcmp = re.match(r"([a-z0-9._+-]+)(.*)", y).groups()
+            dpname, dpvcmp = re.match(r"([a-z0-9._+-]+)(.*)", y).groups()
             if dpname == repopkg.name:
                 logger.warning('Package %s has a depend on itself' % repopkg.name)
                 continue
             dbpkg.packagedepend_set.create(depname=dpname, depvcmp=dpvcmp)
-            logger.debug('Added %s as dep for pkg %s' % (dpname,repopkg.name))
+            logger.debug('Added %s as dep for pkg %s' % (dpname, repopkg.name))
 
     dbpkg.packagegroup_set.all().delete()
     if 'groups' in repopkg.__dict__:
@@ -323,7 +324,7 @@ def db_update(archname, reponame, pkgs, options):
         # for a non-force, we don't want to do anything at all.
         if filesonly:
             pass
-        elif ''.join((p.ver,p.rel)) == ''.join((dbp.pkgver,dbp.pkgrel)):
+        elif '-'.join((p.ver, p.rel)) == '-'.join((dbp.pkgver, dbp.pkgrel)):
             if not force:
                 continue
         else:
@@ -349,16 +350,16 @@ def parse_inf(iofile):
     store = {}
     lines = iofile.readlines()
     blockname = None
-    max = len(lines)
+    max_len = len(lines)
     i = 0
-    while i < max:
+    while i < max_len:
         line = lines[i].strip()
         if len(line) > 0 and line[0] == '%' and line[1:-1].lower() in REPOVARS:
             blockname = line[1:-1].lower()
-            logger.debug("Parsing package block %s",blockname)
+            logger.debug("Parsing package block %s", blockname)
             store[blockname] = []
             i += 1
-            while i < max and len(lines[i].strip()) > 0:
+            while i < max_len and len(lines[i].strip()) > 0:
                 store[blockname].append(lines[i].strip())
                 i += 1
             # here is where i would convert arrays to strings
@@ -402,8 +403,7 @@ def parse_repo(repopath):
             if tpkg != None:
                 tpkg.reset()
                 data = parse_inf(tpkg)
-                p = Pkg(data)
-                p.repo = reponame
+                p = Pkg(data, reponame)
                 logger.debug("Done parsing package %s", p.name)
                 pkgs.append(p)
             if tarinfo == None:
diff --git a/devel/views.py b/devel/views.py
index 192a4572..baa5fd6c 100644
--- a/devel/views.py
+++ b/devel/views.py
@@ -6,19 +6,16 @@ from django.contrib.sites.models import Site
 from django.shortcuts import render_to_response
 from django.template import RequestContext
 from django.core.mail import send_mail
-from django.db.models import Q
 from django.views.decorators.cache import never_cache
 
 from main.models import Package, Todolist, TodolistPkg
 from main.models import Arch, Repo
-from main.models import UserProfile, News
+from main.models import UserProfile
 from main.models import Mirror
 from packages.models import PackageRelation
 
 import random
 from string import ascii_letters, digits
-pwletters = ascii_letters + digits
-
 
 @login_required
 @never_cache
@@ -30,13 +27,16 @@ def index(request):
 
     todopkgs = TodolistPkg.objects.select_related(
             'pkg', 'pkg__arch', 'pkg__repo').filter(complete=False)
-    todopkgs = todopkgs.filter(pkg__pkgbase__in=inner_q).order_by('list__name', 'pkg__pkgname')
+    todopkgs = todopkgs.filter(pkg__pkgbase__in=inner_q).order_by(
+            'list__name', 'pkg__pkgname')
+    maintainers = User.objects.filter(is_active=True).order_by(
+            'first_name', 'last_name')
 
     page_dict = {
             'todos': Todolist.objects.incomplete().order_by('-date_added'),
             'repos': Repo.objects.all(),
             'arches': Arch.objects.all(),
-            'maintainers': User.objects.filter(is_active=True).order_by('first_name', 'last_name'),
+            'maintainers': maintainers,
             'flagged' : flagged,
             'todopkgs' : todopkgs,
          }
@@ -48,9 +48,9 @@ def index(request):
 def change_notify(request):
     maint = User.objects.get(username=request.user.username)
     notify = request.POST.get('notify', 'no')
-    pf = maint.get_profile()
-    pf.notify = (notify == 'yes')
-    pf.save()
+    prof = maint.get_profile()
+    prof.notify = (notify == 'yes')
+    prof.save()
     return HttpResponseRedirect('/devel/')
 
 class ProfileForm(forms.Form):
@@ -98,6 +98,7 @@ class NewUserForm(forms.ModelForm):
 
     def save(self):
         profile = forms.ModelForm.save(self, False)
+        pwletters = ascii_letters + digits
         pw = ''.join([random.choice(pwletters) for i in xrange(8)])
         user = User.objects.create_user(username=self.cleaned_data['username'],
                 email=self.cleaned_data['email'], password=pw)
@@ -125,14 +126,21 @@ def new_user_form(request):
         form = NewUserForm(request.POST)
         if form.is_valid():
             form.save()
-            return HttpResponseRedirect('/admin/auth/user/%d/' %form.instance.user.id)
+            return HttpResponseRedirect('/admin/auth/user/%d/' % \
+                    form.instance.user.id)
     else:
         form = NewUserForm()
-    return render_to_response('general_form.html', RequestContext(
-        request, {'description': '''A new user will be created with the
+
+    context = {
+        'description': '''A new user will be created with the
             following properties in their profile. A random password will be
             generated and the user will be e-mailed with their account details
             n plaintext.''',
-            'form': form, 'title': 'Create User', 'submit_text': 'Create User'}))
+        'form': form,
+        'title': 'Create User',
+        'submit_text': 'Create User'
+    }
+    return render_to_response('general_form.html',
+            RequestContext(request, context))
 
 # vim: set ts=4 sw=4 et:
diff --git a/feeds.py b/feeds.py
index 05c00ca0..37aa6aaa 100644
--- a/feeds.py
+++ b/feeds.py
@@ -1,6 +1,5 @@
 import datetime
 
-from django.contrib.syndication.feeds import FeedDoesNotExist
 from django.contrib.syndication.views import Feed
 from django.db.models import Q
 from main.models import Arch, Repo, Package, News
@@ -53,7 +52,7 @@ class PackageFeed(Feed):
         return item.last_update
 
     def item_categories(self, item):
-        return (item.repo.name,item.arch.name)
+        return (item.repo.name, item.arch.name)
 
 
 class NewsFeed(Feed):
diff --git a/main/models.py b/main/models.py
index c6b4e139..2079deeb 100644
--- a/main/models.py
+++ b/main/models.py
@@ -1,5 +1,4 @@
 from django.db import models
-from django.db.models import Q
 from django.contrib.auth.models import User
 from django.contrib.sites.models import Site
 
@@ -138,7 +137,7 @@ class News(models.Model):
 
 class Arch(models.Model):
     id = models.AutoField(primary_key=True)
-    name = models.CharField(max_length=255,unique=True)
+    name = models.CharField(max_length=255, unique=True)
     def __unicode__(self):
         return self.name
     class Meta:
@@ -148,7 +147,7 @@ class Arch(models.Model):
 
 class Repo(models.Model):
     id = models.AutoField(primary_key=True)
-    name = models.CharField(max_length=255,unique=True)
+    name = models.CharField(max_length=255, unique=True)
     testing = models.BooleanField(default=False,
             help_text="Is this repo meant for package testing?")
     bugs_project = models.SmallIntegerField(default=1,
diff --git a/mirrors/views.py b/mirrors/views.py
index 7ef8becd..378a0b42 100644
--- a/mirrors/views.py
+++ b/mirrors/views.py
@@ -1,5 +1,4 @@
 from django import forms
-from django.db.models import Q
 from django.shortcuts import render_to_response
 from django.template import RequestContext
 from django.views.decorators.csrf import csrf_exempt
diff --git a/packages/views.py b/packages/views.py
index 4fd34edf..2fe515c5 100644
--- a/packages/views.py
+++ b/packages/views.py
@@ -20,20 +20,20 @@ from main.models import Package, PackageFile
 from main.models import Arch, Repo, Signoff
 from main.models import MirrorUrl
 from main.utils import make_choice
-from .models import PackageGroup, PackageRelation
+from .models import PackageRelation
 from .utils import get_group_info, get_differences_info
 
 def opensearch(request):
     if request.is_secure():
-        d = "https://%s" % request.META['HTTP_HOST']
+        domain = "https://%s" % request.META['HTTP_HOST']
     else:
-        d = "http://%s" % request.META['HTTP_HOST']
+        domain = "http://%s" % request.META['HTTP_HOST']
     response = HttpResponse(mimetype='application/opensearchdescription+xml')
-    t = loader.get_template('packages/opensearch.xml')
-    c = Context({
-        'domain': d,
+    template = loader.get_template('packages/opensearch.xml')
+    ctx = Context({
+        'domain': domain,
     })
-    response.write(t.render(c))
+    response.write(template.render(ctx))
     return response
 
 @permission_required('main.change_package')
@@ -54,11 +54,11 @@ def update(request):
         for pkg in pkgs:
             maints = pkg.maintainers
             if mode == 'adopt' and request.user not in maints:
-                pr = PackageRelation(pkgbase=pkg.pkgbase,
+                prel = PackageRelation(pkgbase=pkg.pkgbase,
                         user=request.user,
                         type=PackageRelation.MAINTAINER)
                 count += 1
-                pr.save()
+                prel.save()
             elif mode == 'disown' and request.user in maints:
                 rels = PackageRelation.objects.filter(pkgbase=pkg.pkgbase,
                         user=request.user)
@@ -86,9 +86,9 @@ def details(request, name='', repo='', arch=''):
             arch.lower(), repo.title(), name))
 
 def groups(request):
-    groups = get_group_info()
+    grps = get_group_info()
     return render_to_response('packages/groups.html',
-            RequestContext(request, {'groups': groups}))
+            RequestContext(request, {'groups': grps}))
 
 def group_details(request, arch, name):
     arch = get_object_or_404(Arch, name=arch)
@@ -157,7 +157,7 @@ class PackageSearchForm(forms.Form):
 
 def search(request, page=None):
     current_query = '?'
-    limit=50
+    limit = 50
     packages = Package.objects.select_related('arch', 'repo')
 
     if request.GET:
@@ -224,11 +224,12 @@ def search(request, page=None):
 def files(request, name='', repo='', arch=''):
     pkg = get_object_or_404(Package,
             pkgname=name, repo__name__iexact=repo, arch__name=arch)
-    files = PackageFile.objects.filter(pkg=pkg).order_by('path')
+    fileslist = PackageFile.objects.filter(pkg=pkg).order_by('path')
     template = 'packages/files.html'
     if request.is_ajax():
         template = 'packages/files-list.html'
-    return render_to_response(template, RequestContext(request, {'pkg':pkg,'files':files}))
+    return render_to_response(template, RequestContext(request,
+        {'pkg':pkg, 'files':fileslist}))
 
 @permission_required('main.change_package')
 def unflag(request, name='', repo='', arch=''):
diff --git a/public/utils.py b/public/utils.py
index c92cb264..2801c939 100644
--- a/public/utils.py
+++ b/public/utils.py
@@ -1,4 +1,6 @@
-from main.models import Arch, Repo, Package
+from operator import attrgetter
+
+from main.models import Arch, Package
 from main.utils import cache_function
 
 @cache_function(300)
@@ -7,18 +9,20 @@ def get_recent_updates():
     # want to try and eliminate cross-architecture wasted space. Pull enough
     # packages that we can later do some screening and trim out the fat.
     pkgs = []
-    for a in Arch.objects.all():
+    for arch in Arch.objects.all():
         # grab a few extra so we can hopefully catch everything we need
-        pkgs += list(Package.objects.select_related('arch', 'repo').filter(arch=a).order_by('-last_update')[:50])
+        pkgs += list(Package.objects.select_related(
+            'arch', 'repo').filter(arch=arch).order_by('-last_update')[:50])
     pkgs.sort(key=lambda q: q.last_update)
     updates = []
     ctr = 0
     while ctr < 15 and len(pkgs) > 0:
         # not particularly happy with this logic, but it works.
         p = pkgs.pop()
-        samepkgs = filter(lambda q: p.is_same_version(q) and p.repo == q.repo, pkgs)
+        is_same = lambda q: p.is_same_version(q) and p.repo == q.repo
+        samepkgs = filter(is_same, pkgs)
         samepkgs.append(p)
-        samepkgs.sort(key=lambda q: q.arch.name)
+        samepkgs.sort(key=attrgetter('arch.name'))
         updates.append(samepkgs)
         for q in samepkgs:
             if p != q: pkgs.remove(q)
diff --git a/public/views.py b/public/views.py
index f2c649f9..be0ff80d 100644
--- a/public/views.py
+++ b/public/views.py
@@ -1,5 +1,5 @@
 from main.models import Arch, Donor, MirrorUrl, News
-from main.models import Package, Repo
+from main.models import Repo
 from . import utils
 
 from django.contrib.auth.models import User
diff --git a/urls.py b/urls.py
index 0030343c..3f394426 100644
--- a/urls.py
+++ b/urls.py
@@ -2,9 +2,7 @@ from django.conf.urls.defaults import *
 from django.conf import settings
 from django.contrib import admin
 
-from django.views.generic.create_update import delete_object
 from django.views.generic.simple import direct_to_template
-from django.contrib.auth.decorators import permission_required
 
 from main.models import Todolist
 from feeds import PackageFeed, NewsFeed