[PATCH 04/13] trivial: Don't shadow built-ins
Daniel Axtens
dja at axtens.net
Tue Sep 20 14:47:24 AEST 2016
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 at axtens.net>
Regards,
Daniel
Stephen Finucane <stephenfinucane at 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 at 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*>.*$', '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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 859 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20160920/ec361300/attachment-0001.sig>
More information about the Patchwork
mailing list