Finish linting on py3ish code (#2949)

* Resolve first round of linting errors in restapi.

This is a first step and only adds docstrings, etc. It doesn't refactor any
code.

Conflicts:
	prospector-more.yml
	readthedocs/restapi/views/model_views.py

* Reduce unused params.

In several cases, we can use the provided `request` rather than access `self`
(which could make these functions easier to test in isolation). In others, the
unused args could be folded into an unnamed kwarg param.

* Don't replace section variable.

This worked as intended, but only because the overridden section variable
would later evaluate to True. Using a different variable name makes this a bit
less error-prone.

* Minor docstring update

* Fix indent on docstring

* Punt on refactoring

* Fix call args

* Try unpinning testing reqs

* Bump up prospector higher level strictness

* Linting issues: arguments-differ, len-as-condition, redefined variables

* Remove unnecessary elses

This addresses no-else-return, which removes a `return` in an `else` after an
`if` that already has a `return`. This makes the default return more obvious in
most cases.

* Fix new linting issues with import order

Also, update travis and tox config for multiple versions of python

* Another fix on travis envs

Author: agjohnson

.travis.yml

@@ -1,27 +1,32 @@
 language: python
 python:
- - 3.6
+  - 2.7
+  - 3.6
 sudo: false
-env:
- - TOX_ENV=py27
- - TOX_ENV=py36
- - TOX_ENV=docs
- - TOX_ENV=lint
- - TOX_ENV=eslint
+matrix:
+  include:
+    - python: 2.7
+      env: TOXENV=docs
+    - python: 2.7
+      env: TOXENV=lint
+    - python: 2.7
+      env: TOXENV=eslint
+script:
+  - tox
 cache:
   directories:
     - ~/.cache/pip
     - ~/.nvm/nvm.sh
 install:
- - pip install tox
- - curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.2/install.sh | bash
- - source ~/.nvm/nvm.sh
- - nvm install --lts
- - nvm use --lts
- - npm install
- - bower install
+  - pip install tox-travis
+  - curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.2/install.sh | bash
+  - source ~/.nvm/nvm.sh
+  - nvm install --lts
+  - nvm use --lts
+  - npm install
+  - bower install
 script:
- - tox -e $TOX_ENV
+  - tox
 notifications:
   slack:
     rooms:

prospector-more.yml

@@ -1,9 +1,39 @@
 inherits: prospector
 
-strictness: medium
+strictness: high
 
 ignore-paths:
-  - restapi/
+  - constants.py
+  - urls.py
+  - wsgi.py
+  - acl
+  - api
+  - betterversion
+  - bookmarks
+  - builds
+  - cdn
+  - comments
+  - core
+  - djangome
+  - doc_builder
+  - donate
+  - gold
+  - integrations
+  - locale
+  - notifications
+  - oauth
+  - payments
+  - privacy
+  - profiles
+  - projects
+  - redirects
+  - restapi
+  - rtd_tests
+  - search
+  - settings
+  - tastyapi
+  - templates
+  - vcs_support
 
 pylint:
   options:

readthedocs/api/utils.py

@@ -162,7 +162,7 @@ def is_authenticated(self, request, **kwargs):
 
 
 class EnhancedModelResource(ModelResource):
-    def obj_get_list(self, request=None, *_, **kwargs):
+    def obj_get_list(self, request=None, *_, **kwargs):  # pylint: disable=arguments-differ
         """
         A ORM-specific implementation of ``obj_get_list``.
 

readthedocs/bookmarks/views.py

@@ -25,8 +25,8 @@
 class BookmarkExistsView(View):
 
     @method_decorator(csrf_exempt)
-    def dispatch(self, *args, **kwargs):
-        return super(BookmarkExistsView, self).dispatch(*args, **kwargs)
+    def dispatch(self, request, *args, **kwargs):
+        return super(BookmarkExistsView, self).dispatch(request, *args, **kwargs)
 
     def get(self, request):
         return HttpResponse(
@@ -81,8 +81,8 @@ class BookmarkListView(ListView):
     model = Bookmark
 
     @method_decorator(login_required)
-    def dispatch(self, *args, **kwargs):
-        return super(BookmarkListView, self).dispatch(*args, **kwargs)
+    def dispatch(self, request, *args, **kwargs):
+        return super(BookmarkListView, self).dispatch(request, *args, **kwargs)
 
     def get_queryset(self):
         return Bookmark.objects.filter(user=self.request.user)
@@ -94,8 +94,8 @@ class BookmarkAddView(View):
 
     @method_decorator(login_required)
     @method_decorator(csrf_exempt)
-    def dispatch(self, *args, **kwargs):
-        return super(BookmarkAddView, self).dispatch(*args, **kwargs)
+    def dispatch(self, request, *args, **kwargs):
+        return super(BookmarkAddView, self).dispatch(request, *args, **kwargs)
 
     def get(self, request):
         return HttpResponse(
@@ -157,8 +157,8 @@ class BookmarkRemoveView(View):
 
     @method_decorator(login_required)
     @method_decorator(csrf_exempt)
-    def dispatch(self, *args, **kwargs):
-        return super(BookmarkRemoveView, self).dispatch(*args, **kwargs)
+    def dispatch(self, request, *args, **kwargs):
+        return super(BookmarkRemoveView, self).dispatch(request, *args, **kwargs)
 
     def get(self, request, *args, **kwargs):
         return render_to_response(
@@ -176,30 +176,29 @@ def post(self, request, *args, **kwargs):
             bookmark = get_object_or_404(Bookmark, pk=kwargs['bookmark_pk'])
             bookmark.delete()
             return HttpResponseRedirect(reverse('bookmark_list'))
-        else:
-            try:
-                post_json = json.loads(request.body)
-                project = Project.objects.get(slug=post_json['project'])
-                version = project.versions.get(slug=post_json['version'])
-                url = post_json['url']
-                page = post_json['page']
-            except KeyError:
-                return HttpResponseBadRequest(
-                    json.dumps({'error': "Invalid parameters"})
-                )
-
-            bookmark = get_object_or_404(
-                Bookmark,
-                user=request.user,
-                url=url,
-                project=project,
-                version=version,
-                page=page
+        try:
+            post_json = json.loads(request.body)
+            project = Project.objects.get(slug=post_json['project'])
+            version = project.versions.get(slug=post_json['version'])
+            url = post_json['url']
+            page = post_json['page']
+        except KeyError:
+            return HttpResponseBadRequest(
+                json.dumps({'error': "Invalid parameters"})
             )
-            bookmark.delete()
 
-            return HttpResponse(
-                json.dumps({'removed': True}),
-                status=200,
-                content_type="application/json"
-            )
+        bookmark = get_object_or_404(
+            Bookmark,
+            user=request.user,
+            url=url,
+            project=project,
+            version=version,
+            page=page
+        )
+        bookmark.delete()
+
+        return HttpResponse(
+            json.dumps({'removed': True}),
+            status=200,
+            content_type="application/json"
+        )

readthedocs/builds/forms.py

@@ -33,8 +33,8 @@ class Meta(object):
         model = Version
         fields = ['active', 'privacy_level', 'tags']
 
-    def save(self, *args, **kwargs):
-        obj = super(VersionForm, self).save(*args, **kwargs)
+    def save(self, commit=True):
+        obj = super(VersionForm, self).save(commit=commit)
         if obj.active and not obj.built and not obj.uploaded:
             trigger_build(project=obj.project, version=obj)
         return obj

readthedocs/builds/models.py

@@ -110,8 +110,7 @@ def commit_name(self):
         if self.slug == LATEST:
             if self.project.default_branch:
                 return self.project.default_branch
-            else:
-                return self.project.vcs_repo().fallback_branch
+            return self.project.vcs_repo().fallback_branch
 
         if self.slug == STABLE:
             if self.type == BRANCH:
@@ -148,7 +147,7 @@ def get_absolute_url(self):
         private = self.privacy_level == PRIVATE
         return self.project.get_docs_url(version_slug=self.slug, private=private)
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
         """Add permissions to the Version for all owners on save."""
         from readthedocs.projects import tasks
         obj = super(Version, self).save(*args, **kwargs)
@@ -161,7 +160,7 @@ def save(self, *args, **kwargs):
         broadcast(type='app', task=tasks.symlink_project, args=[self.project.pk])
         return obj
 
-    def delete(self, *args, **kwargs):
+    def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
         from readthedocs.projects import tasks
         log.info('Removing files for version %s', self.slug)
         tasks.clear_artifacts.delay(version_pk=self.pk)

readthedocs/builds/version_slug.py

@@ -17,14 +17,15 @@
 """
 
 from __future__ import absolute_import
-from builtins import range
+
 import math
 import re
 import string
 from operator import truediv
+
 from django.db import models
 from django.utils.encoding import force_text
-from six.moves import range
+from builtins import range
 
 
 # Regex breakdown:

readthedocs/cdn/purge.py

@@ -1,11 +1,12 @@
 """Utility to purge MaxCDN files, if configured."""
 
 from __future__ import absolute_import
-from builtins import range
+
 import logging
 
 from django.conf import settings
-from six.moves import range
+from builtins import range
+
 
 log = logging.getLogger(__name__)
 

readthedocs/comments/models.py

@@ -36,12 +36,12 @@ def from_hash(self, version_slug, page, node_hash, project_slug=None):
         if project_slug:
             snapshots = snapshots.filter(node__project__slug=project_slug)
 
-        if len(snapshots) == 0:
+        if not snapshots.exists():
             raise DocumentNode.DoesNotExist(
                 "No node exists on %s with a current hash of %s" % (
                     page, node_hash))
 
-        if len(snapshots) == 1:
+        if snapshots.count() == 1:
             # If we have found only one snapshot, we know we have the correct node.
             node = snapshots[0].node
         else:
@@ -89,22 +89,20 @@ def latest_commit(self):
     def visible_comments(self):
         if not self.project.comment_moderation:
             return self.comments.all()
-        else:
-            # non-optimal SQL warning.
-            decisions = ModerationAction.objects.filter(
-                comment__node=self,
-                decision=1,
-                date__gt=self.snapshots.latest().date
-            )
-            valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct()
-            return valid_comments
+        # non-optimal SQL warning.
+        decisions = ModerationAction.objects.filter(
+            comment__node=self,
+            decision=1,
+            date__gt=self.snapshots.latest().date
+        )
+        valid_comments = self.comments.filter(moderation_actions__in=decisions).distinct()
+        return valid_comments
 
     def update_hash(self, new_hash, commit):
         latest_snapshot = self.snapshots.latest()
         if latest_snapshot.hash == new_hash and latest_snapshot.commit == commit:
             return latest_snapshot
-        else:
-            return self.snapshots.create(hash=new_hash, commit=commit)
+        return self.snapshots.create(hash=new_hash, commit=commit)
 
 
 class DocumentNodeSerializer(serializers.ModelSerializer):
@@ -190,8 +188,7 @@ def has_been_approved_since_most_recent_node_change(self):
             # If we do have an approval action which is newer than the most recent change,
             # we'll return True or False commensurate with its "approved" attribute.
             return latest_moderation_action.approved()
-        else:
-            return False
+        return False
 
     def is_orphaned(self):
         raise NotImplementedError('TODO')

readthedocs/comments/views.py

@@ -185,7 +185,7 @@ def get_queryset(self):
         return queryset
 
     @method_decorator(login_required)
-    def create(self, request):
+    def create(self, request, *args, **kwargs):
         project = Project.objects.get(slug=request.data['project'])
         comment = project.add_comment(version_slug=request.data['version'],
                                       page=request.data['document_page'],

readthedocs/core/forms.py

@@ -32,11 +32,11 @@ def __init__(self, *args, **kwargs):
         except AttributeError:
             pass
 
-    def save(self, *args, **kwargs):
+    def save(self, commit=True):
         first_name = self.cleaned_data.pop('first_name', None)
         last_name = self.cleaned_data.pop('last_name', None)
-        profile = super(UserProfileForm, self).save(*args, **kwargs)
-        if kwargs.get('commit', True):
+        profile = super(UserProfileForm, self).save(commit=commit)
+        if commit:
             user = profile.user
             user.first_name = first_name
             user.last_name = last_name

readthedocs/core/management/commands/import_github.py

@@ -12,7 +12,7 @@ class Command(BaseCommand):
     help = __doc__
 
     def handle(self, *args, **options):
-        if len(args):
+        if args:
             for slug in args:
                 for service in GitHubService.for_user(
                     User.objects.get(username=slug)

readthedocs/core/management/commands/pull.py

@@ -17,7 +17,7 @@ class Command(BaseCommand):
     help = __doc__
 
     def handle(self, *args, **options):
-        if len(args):
+        if args:
             for slug in args:
                 tasks.update_imported_docs(
                     utils.version_from_slug(slug, LATEST).pk

readthedocs/core/management/commands/update_repos.py

@@ -43,7 +43,7 @@ def handle(self, *args, **options):
         record = options['record']
         force = options['force']
         version = options['version']
-        if len(args):
+        if args:
             for slug in args:
                 if version and version != "all":
                     log.info("Updating version %s for %s", version, slug)