[04/13] trivial: Don't shadow built-ins

* Don't use 'str', 'id', 'max', etc. as variable names
* Remove unnecessary use of six.range
* Remove unnecessary use of six.map

There is one paramter in xmlrpc that cannot be renamed as it could
break some client code. noqa this instead.

Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com>
---
 patchwork/filters.py              | 73 +++++++++++++++++++--------------------
 patchwork/models.py               | 14 ++++----
 patchwork/paginator.py            |  1 -
 patchwork/parser.py               |  9 +++--
 patchwork/templatetags/listurl.py | 14 ++++----
 patchwork/templatetags/person.py  |  4 +--
 patchwork/templatetags/syntax.py  | 23 ++++++------
 patchwork/urls.py                 |  2 +-
 patchwork/views/bundle.py         |  4 +--
 patchwork/views/help.py           |  2 +-
 patchwork/views/patch.py          | 14 ++++----
 patchwork/views/xmlrpc.py         |  2 +-
 12 files changed, 77 insertions(+), 85 deletions(-)

I haven't read through every single litte one of the changes, but I'm
behind the concept. If it passes the tests, I'm pretty happy.

Reviewed-by: Daniel Axtens <dja@axtens.net>

Regards,
Daniel

Stephen Finucane <stephenfinucane@hotmail.com> writes:

> * Don't use 'str', 'id', 'max', etc. as variable names
> * Remove unnecessary use of six.range
> * Remove unnecessary use of six.map
>
> There is one paramter in xmlrpc that cannot be renamed as it could
> break some client code. noqa this instead.
>
> Signed-off-by: Stephen Finucane <stephenfinucane@hotmail.com>
> ---
>  patchwork/filters.py              | 73 +++++++++++++++++++--------------------
>  patchwork/models.py               | 14 ++++----
>  patchwork/paginator.py            |  1 -
>  patchwork/parser.py               |  9 +++--
>  patchwork/templatetags/listurl.py | 14 ++++----
>  patchwork/templatetags/person.py  |  4 +--
>  patchwork/templatetags/syntax.py  | 23 ++++++------
>  patchwork/urls.py                 |  2 +-
>  patchwork/views/bundle.py         |  4 +--
>  patchwork/views/help.py           |  2 +-
>  patchwork/views/patch.py          | 14 ++++----
>  patchwork/views/xmlrpc.py         |  2 +-
>  12 files changed, 77 insertions(+), 85 deletions(-)
>
> diff --git a/patchwork/filters.py b/patchwork/filters.py
> index 5650014..bea4340 100644
> --- a/patchwork/filters.py
> +++ b/patchwork/filters.py
> @@ -55,10 +55,13 @@ class Filter(object):
>             that user's delegated patches"""
>          pass
>  
> -    def parse(self, dict):
> -        if self.param not in dict:
> +    def _set_key(self, key):
> +        raise NotImplementedError
> +
> +    def parse(self, values):
> +        if self.param not in values:
>              return
> -        self._set_key(dict[self.param])
> +        self._set_key(values[self.param])
>  
>      def url_without_me(self):
>          return self.filters.querystring_without_filter(self)
> @@ -89,34 +92,29 @@ class SubmitterFilter(Filter):
>          self.person = None
>          self.person_match = None
>  
> -    def _set_key(self, str):
> +    def _set_key(self, key):
>          self.person = None
>          self.person_match = None
>          submitter_id = None
>  
> -        str = str.strip()
> -        if str == '':
> +        key = key.strip()
> +        if not key:
>              return
>  
>          try:
> -            submitter_id = int(str)
> +            submitter_id = int(key)
>          except ValueError:
>              pass
> -        except:
> -            return
>  
>          if submitter_id:
> -            self.person = Person.objects.get(id=int(str))
> +            self.person = Person.objects.get(id=submitter_id)
>              self.applied = True
>              return
>  
> -        people = Person.objects.filter(name__icontains=str)
> -
> -        if not people:
> -            return
> -
> -        self.person_match = str
> -        self.applied = True
> +        people = Person.objects.filter(name__icontains=key)
> +        if people:
> +            self.person_match = key
> +            self.applied = True
>  
>      def kwargs(self):
>          if self.person:
> @@ -158,16 +156,16 @@ class StateFilter(Filter):
>          self.state = None
>          self.applied = True
>  
> -    def _set_key(self, str):
> +    def _set_key(self, key):
>          self.state = None
>  
> -        if str == self.any_key:
> +        if key == self.any_key:
>              self.applied = False
>              return
>  
>          try:
> -            self.state = State.objects.get(id=int(str))
> -        except:
> +            self.state = State.objects.get(id=int(key))
> +        except ValueError, State.DoesNotExist:
>              return
>  
>          self.applied = True
> @@ -193,17 +191,17 @@ class StateFilter(Filter):
>          return None
>  
>      def _form(self):
> -        str = '<select name="%s" class="form-control">' % self.param
> +        out = '<select name="%s" class="form-control">' % self.param
>  
>          selected = ''
>          if not self.applied:
>              selected = 'selected'
> -        str += '<option %s value="%s">any</option>' % (selected, self.any_key)
> +        out += '<option %s value="%s">any</option>' % (selected, self.any_key)
>  
>          selected = ''
>          if self.applied and self.state is None:
>              selected = 'selected'
> -        str += '<option %s value="">%s</option>' % (
> +        out += '<option %s value="">%s</option>' % (
>              selected, self.action_req_str)
>  
>          for state in State.objects.all():
> @@ -211,10 +209,10 @@ class StateFilter(Filter):
>              if self.state and self.state == state:
>                  selected = ' selected="true"'
>  
> -            str += '<option value="%d" %s>%s</option>' % (
> +            out += '<option value="%d" %s>%s</option>' % (
>                  state.id, selected, state.name)
> -        str += '</select>'
> -        return mark_safe(str)
> +        out += '</select>'
> +        return mark_safe(out)
>  
>      def form_function(self):
>          return 'function(form) { return form.x.value }'
> @@ -235,11 +233,12 @@ class SearchFilter(Filter):
>          self.param = 'q'
>          self.search = None
>  
> -    def _set_key(self, str):
> -        str = str.strip()
> -        if str == '':
> +    def _set_key(self, key):
> +        key = key.strip()
> +        if not key:
>              return
> -        self.search = str
> +
> +        self.search = key
>          self.applied = True
>  
>      def kwargs(self):
> @@ -281,11 +280,11 @@ class ArchiveFilter(Filter):
>              None: 'Both'
>          }
>  
> -    def _set_key(self, str):
> +    def _set_key(self, key):
>          self.archive_state = False
>          self.applied = True
>          for (k, v) in self.param_map.items():
> -            if str == v:
> +            if key == v:
>                  self.archive_state = k
>          if self.archive_state is None:
>              self.applied = False
> @@ -351,8 +350,6 @@ class DelegateFilter(Filter):
>              delegate_id = int(key)
>          except ValueError:
>              pass
> -        except:
> -            return
>  
>          if delegate_id:
>              self.delegate = User.objects.get(id=int(key))
> @@ -410,11 +407,11 @@ class Filters:
>  
>      def __init__(self, request):
>          self._filters = [c(self) for c in filterclasses]
> -        self.dict = request.GET
> +        self.values = request.GET
>          self.project = None
>  
>          for f in self._filters:
> -            f.parse(self.dict)
> +            f.parse(self.values)
>  
>      def set_project(self, project):
>          self.project = project
> @@ -439,7 +436,7 @@ class Filters:
>      def querystring(self, remove=None):
>          params = dict(self.params())
>  
> -        for (k, v) in self.dict.items():
> +        for (k, v) in self.values.items():
>              if k not in params:
>                  params[k] = v
>  
> diff --git a/patchwork/models.py b/patchwork/models.py
> index 5043c9a..b1c6d8d 100644
> --- a/patchwork/models.py
> +++ b/patchwork/models.py
> @@ -33,7 +33,6 @@ from django.core.urlresolvers import reverse
>  from django.db import models
>  from django.utils.encoding import python_2_unicode_compatible
>  from django.utils.functional import cached_property
> -from django.utils.six.moves import filter
>  
>  from patchwork.fields import HashField
>  
> @@ -136,9 +135,8 @@ class UserProfile(models.Model):
>  
>      def name(self):
>          if self.user.first_name or self.user.last_name:
> -            names = list(filter(
> -                bool, [self.user.first_name, self.user.last_name]))
> -            return ' '.join(names)
> +            names = [self.user.first_name, self.user.last_name]
> +            return ' '.join([x for x in names if x])
>          return self.user.username
>  
>      def contributor_projects(self):
> @@ -687,10 +685,10 @@ class EmailConfirmation(models.Model):
>          return self.date + self.validity > datetime.datetime.now()
>  
>      def save(self):
> -        max = 1 << 32
> -        if self.key == '':
> -            str = '%s%s%d' % (self.user, self.email, random.randint(0, max))
> -            self.key = self._meta.get_field('key').construct(str).hexdigest()
> +        limit = 1 << 32
> +        if not self.key:
> +            key = '%s%s%d' % (self.user, self.email, random.randint(0, limit))
> +            self.key = self._meta.get_field('key').construct(key).hexdigest()
>          super(EmailConfirmation, self).save()
>  
>  
> diff --git a/patchwork/paginator.py b/patchwork/paginator.py
> index 5ae0346..e31c76c 100644
> --- a/patchwork/paginator.py
> +++ b/patchwork/paginator.py
> @@ -21,7 +21,6 @@ from __future__ import absolute_import
>  
>  from django.conf import settings
>  from django.core import paginator
> -from django.utils.six.moves import range
>  
>  
>  DEFAULT_ITEMS_PER_PAGE = 100
> diff --git a/patchwork/parser.py b/patchwork/parser.py
> index 1805df8..82f2d5e 100644
> --- a/patchwork/parser.py
> +++ b/patchwork/parser.py
> @@ -31,7 +31,6 @@ import re
>  
>  from django.contrib.auth.models import User
>  from django.utils import six
> -from django.utils.six.moves import map
>  
>  from patchwork.models import (Patch, Project, Person, Comment, State,
>                                DelegationRule, Submission, CoverLetter,
> @@ -45,9 +44,9 @@ list_id_headers = ['List-ID', 'X-Mailing-List', 'X-list']
>  LOGGER = logging.getLogger(__name__)
>  
>  
> -def normalise_space(str):
> +def normalise_space(value):
>      whitespace_re = re.compile(r'\s+')
> -    return whitespace_re.sub(' ', str).strip()
> +    return whitespace_re.sub(' ', value).strip()
>  
>  
>  def clean_header(header):
> @@ -60,7 +59,7 @@ def clean_header(header):
>              return frag_str.decode()
>          return frag_str
>  
> -    fragments = list(map(decode, decode_header(header)))
> +    fragments = [decode(x) for x in decode_header(header)]
>  
>      return normalise_space(u' '.join(fragments))
>  
> @@ -454,7 +453,7 @@ def parse_patch(content):
>                          return 1
>                      return int(x)
>  
> -                lc = list(map(fn, match.groups()))
> +                lc = [fn(x) for x in match.groups()]
>  
>                  state = 4
>                  patchbuf += buf + line
> diff --git a/patchwork/templatetags/listurl.py b/patchwork/templatetags/listurl.py
> index 7e0e3d7..3f28f71 100644
> --- a/patchwork/templatetags/listurl.py
> +++ b/patchwork/templatetags/listurl.py
> @@ -47,18 +47,18 @@ class ListURLNode(template.defaulttags.URLNode):
>          view_name = template.Variable('list_view.view').resolve(context)
>          kwargs = template.Variable('list_view.view_params').resolve(context)
>  
> -        str = None
> +        path = None
>          try:
> -            str = reverse(view_name, args=[], kwargs=kwargs)
> +            path = reverse(view_name, args=[], kwargs=kwargs)
>          except NoReverseMatch:
>              try:
>                  project_name = settings.SETTINGS_MODULE.split('.')[0]
> -                str = reverse(project_name + '.' + view_name,
> -                              args=[], kwargs=kwargs)
> +                path = reverse(project_name + '.' + view_name,
> +                               args=[], kwargs=kwargs)
>              except NoReverseMatch:
>                  raise
>  
> -        if str is None:
> +        if path is None:
>              return ''
>  
>          params = []
> @@ -72,9 +72,9 @@ class ListURLNode(template.defaulttags.URLNode):
>              params[smart_str(k, 'ascii')] = v.resolve(context)
>  
>          if not params:
> -            return str
> +            return path
>  
> -        return str + '?' + '&'.join(
> +        return path + '?' + '&'.join(
>              ['%s=%s' % (k, escape(v)) for (k, v) in list(params.items())])
>  
>  
> diff --git a/patchwork/templatetags/person.py b/patchwork/templatetags/person.py
> index 7af021f..adbabb0 100644
> --- a/patchwork/templatetags/person.py
> +++ b/patchwork/templatetags/person.py
> @@ -40,7 +40,7 @@ def personify(person, project):
>  
>      url = reverse('patch-list',
>                    kwargs={'project_id': project.linkname})
> -    str = '<a href="%s?%s=%s">%s</a>' % (
> +    out = '<a href="%s?%s=%s">%s</a>' % (
>          url, SubmitterFilter.param, escape(person.id), linktext)
>  
> -    return mark_safe(str)
> +    return mark_safe(out)
> diff --git a/patchwork/templatetags/syntax.py b/patchwork/templatetags/syntax.py
> index 6cb8ff8..51bd787 100644
> --- a/patchwork/templatetags/syntax.py
> +++ b/patchwork/templatetags/syntax.py
> @@ -24,27 +24,26 @@ import re
>  from django import template
>  from django.utils.html import escape
>  from django.utils.safestring import mark_safe
> -from django.utils.six.moves import map
>  
>  
>  register = template.Library()
>  
>  
> -def _compile(t):
> -    (r, str) = t
> -    return (re.compile(r, re.M | re.I), str)
> +def _compile(value):
> +    regex, cls = value
> +    return re.compile(regex, re.M | re.I), cls
>  
> -_patch_span_res = list(map(_compile, [
> +_patch_span_res = [_compile(x) for x in [
>      ('^(Index:?|diff|\-\-\-|\+\+\+|\*\*\*) .*$', 'p_header'),
>      ('^\+.*$', 'p_add'),
>      ('^-.*$', 'p_del'),
>      ('^!.*$', 'p_mod'),
> -]))
> +]]
>  
> -_patch_chunk_re = \
> -    re.compile('^(@@ \-\d+(?:,\d+)? \+\d+(?:,\d+)? @@)(.*)$', re.M | re.I)
> +_patch_chunk_re = re.compile(
> +    '^(@@ \-\d+(?:,\d+)? \+\d+(?:,\d+)? @@)(.*)$', re.M | re.I)
>  
> -_comment_span_res = list(map(_compile, [
> +_comment_span_res = [_compile(x) for x in [
>      ('^\s*Signed-off-by: .*$', 'signed-off-by'),
>      ('^\s*Acked-by: .*$', 'acked-by'),
>      ('^\s*Nacked-by: .*$', 'nacked-by'),
> @@ -52,7 +51,7 @@ _comment_span_res = list(map(_compile, [
>      ('^\s*Reviewed-by: .*$', 'reviewed-by'),
>      ('^\s*From: .*$', 'from'),
>      ('^\s*&gt;.*$', 'quote'),
> -]))
> +]]
>  
>  _span = '<span class="%s">%s</span>'
>  
> @@ -61,8 +60,8 @@ _span = '<span class="%s">%s</span>'
>  def patchsyntax(patch):
>      diff = escape(patch.diff).replace('\r\n', '\n')
>  
> -    for (r, cls) in _patch_span_res:
> -        diff = r.sub(lambda x: _span % (cls, x.group(0)), diff)
> +    for (regex, cls) in _patch_span_res:
> +        diff = regex.sub(lambda x: _span % (cls, x.group(0)), diff)
>  
>      diff = _patch_chunk_re.sub(
>          lambda x:
> diff --git a/patchwork/urls.py b/patchwork/urls.py
> index 39248ea..30c6c73 100644
> --- a/patchwork/urls.py
> +++ b/patchwork/urls.py
> @@ -124,7 +124,7 @@ urlpatterns = [
>      url(r'^mail/optin/$', mail_views.optin, name='mail-optin'),
>  
>      # help!
> -    url(r'^help/(?P<path>.*)$', help_views.help, name='help'),
> +    url(r'^help/(?P<path>.*)$', help_views.detail, name='help'),
>  ]
>  
>  if 'debug_toolbar' in settings.INSTALLED_APPS:
> diff --git a/patchwork/views/bundle.py b/patchwork/views/bundle.py
> index dabaef1..ba569e2 100644
> --- a/patchwork/views/bundle.py
> +++ b/patchwork/views/bundle.py
> @@ -65,9 +65,9 @@ def setbundle(request):
>              else:
>                  patch_ids = get_patch_ids(request.POST)
>  
> -            for id in patch_ids:
> +            for patch_id in patch_ids:
>                  try:
> -                    patch = Patch.objects.get(id=id)
> +                    patch = Patch.objects.get(id=patch_id)
>                      bundle.append_patch(patch)
>                  except:
>                      pass
> diff --git a/patchwork/views/help.py b/patchwork/views/help.py
> index 1d42946..7666b0a 100644
> --- a/patchwork/views/help.py
> +++ b/patchwork/views/help.py
> @@ -33,7 +33,7 @@ if settings.ENABLE_XMLRPC:
>      help_pages['pwclient/'] = 'pwclient.html'
>  
>  
> -def help(request, path):
> +def detail(request, path):
>      if path in help_pages:
>          return render(request,
>                        'patchwork/help/' + help_pages[path])
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index 3346568..41a2ec8 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -108,6 +108,13 @@ def patch(request, patch_id):
>      return render(request, 'patchwork/submission.html', context)
>  
>  
> +def patches(request, project_id):
> +    project = get_object_or_404(Project, linkname=project_id)
> +    context = generic_list(request, project, 'patch-list',
> +                           view_args={'project_id': project.linkname})
> +    return render(request, 'patchwork/list.html', context)
> +
> +
>  def content(request, patch_id):
>      patch = get_object_or_404(Patch, id=patch_id)
>      response = HttpResponse(content_type="text/x-patch")
> @@ -128,10 +135,3 @@ def mbox(request, patch_id):
>      response['Content-Disposition'] = 'attachment; filename=' + \
>          patch.filename().replace(';', '').replace('\n', '')
>      return response
> -
> -
> -def list(request, project_id):
> -    project = get_object_or_404(Project, linkname=project_id)
> -    context = generic_list(request, project, 'patch-list',
> -                           view_args={'project_id': project.linkname})
> -    return render(request, 'patchwork/list.html', context)
> diff --git a/patchwork/views/xmlrpc.py b/patchwork/views/xmlrpc.py
> index 8f950a6..ea2a1e3 100644
> --- a/patchwork/views/xmlrpc.py
> +++ b/patchwork/views/xmlrpc.py
> @@ -635,7 +635,7 @@ def patch_get(patch_id):
>  
>  
>  @xmlrpc_method()
> -def patch_get_by_hash(hash):
> +def patch_get_by_hash(hash):  # noqa
>      """Get a patch by its hash.
>  
>      Retrieve a patch matching a given patch hash, if any exists.
> -- 
> 2.7.4
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

On 20 Sep 14:47, Daniel Axtens wrote:
> I haven't read through every single litte one of the changes, but I'm
> behind the concept. If it passes the tests, I'm pretty happy.
> 
> Reviewed-by: Daniel Axtens <dja@axtens.net>

Thanks. Merged.