added pylint

Added pylintrc and pylinted the current code.

Other notes:
-added pylint==1.3.0

edxval/__init__.py

+"""
+init
+"""

edxval/admin.py

+"""
+Admin file for django app edxval.
+"""
+
 from django.contrib import admin
 from .models import Video, Profile, EncodedVideo
 

edxval/api.py

@@ -7,7 +7,7 @@ import logging
 from edxval.models import Video
 from edxval.serializers import VideoSerializer
 
-logger = logging.getLogger(__name__)
+logger = logging.getLogger(__name__) # pylint: disable=C0103
 
 
 class ValError(Exception):
@@ -44,7 +44,7 @@ class ValVideoNotFoundError(ValError):
     pass
 
 
-def get_video_info(edx_video_id, location=None):
+def get_video_info(edx_video_id, location=None): # pylint: disable=W0613
     """
     Retrieves all encoded videos of a video found with given video edx_video_id
 
@@ -95,8 +95,8 @@ def get_video_info(edx_video_id, location=None):
         }
     """
     try:
-        v = Video.objects.get(edx_video_id=edx_video_id)
-        result = VideoSerializer(v)
+        video = Video.objects.get(edx_video_id=edx_video_id)
+        result = VideoSerializer(video)
     except Video.DoesNotExist:
         error_message = u"Video not found for edx_video_id: {0}".format(edx_video_id)
         raise ValVideoNotFoundError(error_message)
@@ -104,4 +104,4 @@ def get_video_info(edx_video_id, location=None):
         error_message = u"Could not get edx_video_id: {0}".format(edx_video_id)
         logger.exception(error_message)
         raise ValInternalError(error_message)
-    return result.data
+    return result.data # pylint: disable=E1101

edxval/models.py

@@ -31,7 +31,7 @@ class Video(models.Model):
         unique=True,
         validators=[
             RegexValidator(
-                regex='^[a-zA-Z0-9\-]*$',
+                regex=r'^[a-zA-Z0-9\-]*$',
                 message='edx_video_id has invalid characters',
                 code='invalid edx_video_id'
             ),
@@ -51,7 +51,10 @@ class CourseVideos(models.Model):
     course_id = models.CharField(max_length=255)
     video = models.ForeignKey(Video)
 
-    class Meta:
+    class Meta: # pylint: disable=C1001
+        """
+        course_id is listed first in this composite index
+        """
         unique_together = ("course_id", "video")
 
 

edxval/serializers.py

@@ -8,7 +8,10 @@ from edxval.models import Profile, Video, EncodedVideo
 
 
 class ProfileSerializer(serializers.ModelSerializer):
-    class Meta:
+    """
+    Serializer for Profile object.
+    """
+    class Meta: # pylint: disable=C1001, C0111
         model = Profile
         fields = (
             "profile_name",
@@ -19,9 +22,14 @@ class ProfileSerializer(serializers.ModelSerializer):
 
 
 class EncodedVideoSerializer(serializers.ModelSerializer):
+    """
+    Serializer for EncodedVideo object.
+
+    Uses the profile_name as it's profile value instead of a Profile object.
+    """
     profile = serializers.SlugRelatedField(slug_field="profile_name")
 
-    class Meta:
+    class Meta: # pylint: disable=C1001, C0111
         model = EncodedVideo
         fields = (
             "created",
@@ -41,12 +49,14 @@ class EncodedVideoSerializer(serializers.ModelSerializer):
 
 
 class VideoSerializer(serializers.HyperlinkedModelSerializer):
-    encoded_videos = EncodedVideoSerializer(
-        many=True,
-        allow_add_remove=True
-    )
+    """
+    Serializer for Video object
+
+    encoded_videos takes a list of dicts EncodedVideo data.
+    """
+    encoded_videos = EncodedVideoSerializer(many=True, allow_add_remove=True)
 
-    class Meta:
+    class Meta: # pylint: disable=C1001,C0111
         model = Video
         lookup_field = "edx_video_id"
 

edxval/settings.py

-# Django settings for edxval project.
+"""
+Settings file for django app edxval.
+"""
 
 DEBUG = True
 TEMPLATE_DEBUG = DEBUG
@@ -78,7 +80,7 @@ STATICFILES_DIRS = (
 STATICFILES_FINDERS = (
     'django.contrib.staticfiles.finders.FileSystemFinder',
     'django.contrib.staticfiles.finders.AppDirectoriesFinder',
-#    'django.contrib.staticfiles.finders.DefaultStorageFinder',
+    # 'django.contrib.staticfiles.finders.DefaultStorageFinder',
 )
 
 # Make this unique, and don't share it with anybody.
@@ -88,7 +90,7 @@ SECRET_KEY = ')5n@d^*763&##4c(vtzg6&%d7^yiee@5zk-n$rw7djcmz+4u4n'
 TEMPLATE_LOADERS = (
     'django.template.loaders.filesystem.Loader',
     'django.template.loaders.app_directories.Loader',
-#     'django.template.loaders.eggs.Loader',
+    # 'django.template.loaders.eggs.Loader',
 )
 
 MIDDLEWARE_CLASSES = (

edxval/tests/__init__.py

+"""
+init
+"""

edxval/tests/constants.py

 # -*- coding: utf-8 -*-
+"""
+Constants used for tests.
+"""
 EDX_VIDEO_ID = "itchyjacket"
 """
 Generic Profiles for manually creating profile objects

edxval/tests/test_api.py

@@ -18,6 +18,9 @@ from edxval.tests import constants
 
 
 class GetVideoInfoTest(TestCase):
+    """
+    Tests for our get_video_indo function in api.py
+    """
 
     def setUp(self):
         """
@@ -45,8 +48,10 @@ class GetVideoInfoTest(TestCase):
         """
         Tests for successful video request
         """
-        self.assertIsNotNone(api.get_video_info(
-            constants.VIDEO_DICT_FISH.get("edx_video_id"))
+        self.assertIsNotNone(
+            api.get_video_info(
+                constants.VIDEO_DICT_FISH.get("edx_video_id")
+            )
         )
 
     def test_no_such_video(self):
@@ -90,6 +95,9 @@ class GetVideoInfoTest(TestCase):
 
 
 class GetVideoInfoTestWithHttpCalls(APITestCase):
+    """
+    Tests for the get_info_video, using the HTTP requests to populate database
+    """
 
     def setUp(self):
         """

edxval/tests/test_serializers.py

@@ -32,14 +32,13 @@ class SerializerTests(TestCase):
 
         Tests negative inputs for bitrate, file_size in EncodedVideo
         """
-        a = EncodedVideoSerializer(
+        errors = EncodedVideoSerializer( # pylint: disable=E1101
             data=constants.ENCODED_VIDEO_DICT_NEGATIVE_BITRATE).errors
-        self.assertEqual(a.get('bitrate')[0],
+        self.assertEqual(errors.get('bitrate')[0],
                          u"Ensure this value is greater than or equal to 0.")
-        b = EncodedVideoSerializer(
-
+        errors = EncodedVideoSerializer( # pylint: disable=E1101
             data=constants.ENCODED_VIDEO_DICT_NEGATIVE_FILESIZE).errors
-        self.assertEqual(b.get('file_size')[0],
+        self.assertEqual(errors.get('file_size')[0],
                          u"Ensure this value is greater than or equal to 0.")
 
     def test_negative_fields_for_video_serializer(self):
@@ -48,9 +47,9 @@ class SerializerTests(TestCase):
 
         Tests negative inputs for duration in model Video
         """
-        c = VideoSerializer(
+        errors = VideoSerializer( # pylint: disable=E1101
             data=constants.VIDEO_DICT_NEGATIVE_DURATION).errors
-        self.assertEqual(c.get('duration')[0],
+        self.assertEqual(errors.get('duration')[0],
                          u"Ensure this value is greater than or equal to 0.")
 
     def test_non_latin_serialization(self):
@@ -67,7 +66,7 @@ class SerializerTests(TestCase):
         """
         Test the Video model regex validation for edx_video_id field
         """
-        error = VideoSerializer(data=constants.VIDEO_DICT_INVALID_ID).errors
+        error = VideoSerializer(data=constants.VIDEO_DICT_INVALID_ID).errors # pylint: disable=E1101
         message = error.get("edx_video_id")[0]
         self.assertEqual(
             message,
@@ -88,7 +87,7 @@ class SerializerTests(TestCase):
             profile=Profile.objects.get(profile_name="mobile"),
             **constants.ENCODED_VIDEO_DICT_MOBILE
         )
-        result = VideoSerializer(video).data
+        result = VideoSerializer(video).data # pylint: disable=E1101
         # Check for 2 EncodedVideo entries
         self.assertEqual(len(result.get("encoded_videos")), 2)
         # Check for original Video data

edxval/tests/test_views.py

+ # pylint: disable=E1103, W0106
+"""
+Tests for Video Abstraction Layer views
+"""
 from django.core.urlresolvers import reverse
 from rest_framework import status
 from rest_framework.test import APITestCase
@@ -18,11 +22,8 @@ class VideoDetail(APITestCase):
         Profile.objects.create(**constants.PROFILE_DICT_MOBILE)
         Profile.objects.create(**constants.PROFILE_DICT_DESKTOP)
 
-    """
-    Tests for successful PUT requests.
+    #Tests for successful PUT requests.
 
-    These tests should be returning HTTP_200_OK responses.
-    """
     def test_update_video(self):
         """
         Tests PUTting a single video with no encoded videos.
@@ -90,7 +91,7 @@ class VideoDetail(APITestCase):
             'video-detail',
             kwargs={"edx_video_id": constants.COMPLETE_SET_FISH.get("edx_video_id")}
         )
-        response = self.client.patch(
+        response = self.client.patch( # pylint: disable=E1101
             path=url,
             data=constants.COMPLETE_SET_UPDATE_FISH,
             format='json'
@@ -215,11 +216,7 @@ class VideoDetail(APITestCase):
             constants.ENCODED_VIDEO_DICT_UPDATE_FISH_DESKTOP.get("url")
         )
 
-    """
-    Tests for bad PUT requests.
-
-    These tests should be returning HTTP_400_BAD_REQUEST responses.
-    """
+    #Tests for bad PUT requests.
 
     def test_update_an_invalid_encoded_videos(self):
         """
@@ -303,15 +300,11 @@ class VideoListTest(APITestCase):
         Profile.objects.create(**constants.PROFILE_DICT_MOBILE)
         Profile.objects.create(**constants.PROFILE_DICT_DESKTOP)
 
-    """
-    Tests for successful POST requests.
-
-    These tests should be returning HTTP_201_CREATED responses.
-    """
+    # Tests for successful POST 201 requests.
     def test_complete_set_two_encoded_video_post(self):
         """
         Tests POSTing Video and EncodedVideo pair
-        """
+        """ #pylint: disable=R0801
         url = reverse('video-list')
         response = self.client.post(
             url, constants.COMPLETE_SET_FISH, format='json'
@@ -357,25 +350,8 @@ class VideoListTest(APITestCase):
         response = self.client.post(url, constants.VIDEO_DICT_NON_LATIN_TITLE, format='json')
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-    def test_post_video_with_duplicate_encoded_videos(self):
-        """
-        Tests POSTing a single video with duplicate EncodedVideos
-        """
-        url = reverse('video-list')
-        response = self.client.post(url, constants.COMPLETE_SET_TWO_MOBILE_FISH, format='json')
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        video = self.client.get("/edxval/video/").data
-        self.assertEqual(len(video), 0)
-        self.assertEqual(
-            response.data.get("non_field_errors")[0],
-            "Invalid data: duplicate profiles"
-        )
-
-    """
-    Tests for POSTing invalid data
+    # Tests for 400_bad_request POSTs
 
-    These tests should be returning HTTP_400_BAD_REQUEST
-    """
     def test_post_videos(self):
         """
         Tests POSTing same video.
@@ -451,9 +427,8 @@ class VideoListTest(APITestCase):
             errors.get("edx_video_id")[0],
             "edx_video_id has invalid characters"
         )
-    """
-    Tests for number of queries
-    """
+    # Tests for POST queries to database
+
     def test_queries_for_only_video(self):
         """
         Tests number of queries for a Video with no Encoded Videos
@@ -478,6 +453,30 @@ class VideoListTest(APITestCase):
         with self.assertNumQueries(7):
             self.client.post(url, constants.COMPLETE_SET_STAR, format='json')
 
+
+class VideoDetailTest(APITestCase):
+    """
+    Tests for GET
+    """
+    def setUp(self):
+        """
+        Used for manually creating profile objects which EncodedVideos require.
+        """
+        Profile.objects.create(**constants.PROFILE_DICT_MOBILE)
+        Profile.objects.create(**constants.PROFILE_DICT_DESKTOP)
+
+    def test_get_all_videos(self):
+        """
+        Tests getting all Video objects
+        """
+        url = reverse('video-list')
+        response = self.client.post(url, constants.VIDEO_DICT_ANIMAL, format='json')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        response = self.client.post(url, constants.VIDEO_DICT_ZEBRA, format='json')
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        videos = self.client.get("/edxval/video/").data
+        self.assertEqual(len(videos), 2)
+
     def test_queries_for_get(self):
         """
         Tests number of queries when GETting all videos
@@ -497,20 +496,3 @@ class VideoListTest(APITestCase):
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         with self.assertNumQueries(5):
             self.client.get("/edxval/video/").data
-    """
-    Tests for GET
-    """
-    def test_get_all_videos(self):
-        """
-        Tests getting all Video objects
-        """
-        url = reverse('video-list')
-        response = self.client.post(url, constants.VIDEO_DICT_ANIMAL, format='json')
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        response = self.client.post(url, constants.VIDEO_DICT_ZEBRA, format='json')
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        videos = self.client.get("/edxval/video/").data
-        self.assertEqual(len(videos), 2)
-
-
-

edxval/urls.py

+"""
+Url file for django app edxval.
+"""
+
 from django.conf.urls import patterns, include, url
-from rest_framework.urlpatterns import format_suffix_patterns
 
 from edxval import views
 
 from django.contrib import admin
 admin.autodiscover()
 
-urlpatterns = patterns('',
+urlpatterns = patterns(
+    '',
     url(r'^edxval/video/$', views.VideoList.as_view(),
         name="video-list"),
     url(r'^edxval/video/(?P<edx_video_id>[-\w]+)',

edxval/views.py

+"""
+Views file for django app edxval.
+"""
+
 from rest_framework import generics
 
 from edxval.models import Video, Profile

edxval/wsgi.py

@@ -21,7 +21,7 @@ os.environ.setdefault("DJANGO_SETTINGS_MODULE", "edxval.settings")
 # file. This includes Django's development server, if the WSGI_APPLICATION
 # setting points here.
 from django.core.wsgi import get_wsgi_application
-application = get_wsgi_application()
+application = get_wsgi_application() # pylint: disable=C0103
 
 # Apply WSGI middleware here.
 # from helloworld.wsgi import HelloWorldApplication

pylintrc

+[MASTER]
+
+# Specify a configuration file.
+#rcfile=
+
+# Python code to execute, usually for sys.path manipulation such as
+# pygtk.require().
+#init-hook=
+
+# Profiled execution.
+profile=no
+
+# Add files or directories to the blacklist. They should be base names, not
+# paths.
+ignore=CVS, migrations
+
+# Pickle collected data for later comparisons.
+persistent=yes
+
+# List of plugins (as comma separated values of python modules names) to load,
+# usually to register additional checkers.
+load-plugins=
+
+
+[MESSAGES CONTROL]
+
+# Enable the message, report, category or checker with the given id(s). You can
+# either give multiple identifier separated by comma (,) or put this option
+# multiple time.
+#enable=
+
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifier separated by comma (,) or put this option
+# multiple time (only on the command line, not in the configuration file where
+# it should appear only once).
+disable=
+# Never going to use these
+# I0011: Locally disabling W0232
+# C0301: Line too long
+# W0141: Used builtin function 'map'
+# W0142: Used * or ** magic
+# R0921: Abstract class not referenced
+# R0922: Abstract class is only referenced 1 times
+    I0011,C0301,W0141,W0142,R0921,R0922,
+
+# Django makes classes that trigger these
+# W0232: Class has no __init__ method
+    W0232,
+
+# Might use these when the code is in better shape
+# C0302: Too many lines in module
+# R0201: Method could be a function
+# R0901: Too many ancestors
+# R0902: Too many instance attributes
+# R0903: Too few public methods (1/2)
+# R0904: Too many public methods
+# R0911: Too many return statements
+# R0912: Too many branches
+# R0913: Too many arguments
+# R0914: Too many local variables
+    C0302,R0201,R0901,R0902,R0903,R0904,R0911,R0912,R0913,R0914
+
+
+[REPORTS]
+
+# Set the output format. Available formats are text, parseable, colorized, msvs
+# (visual studio) and html
+output-format=text
+
+# Include message's id in output
+include-ids=yes
+
+# Put messages in a separate file for each module / package specified on the
+# command line instead of printing them on stdout. Reports (if any) will be
+# written in a file name "pylint_global.[txt|html]".
+files-output=no
+
+# Tells whether to display a full report or only the messages
+reports=no
+
+# Python expression which should return a note less than 10 (10 is the highest
+# note). You have access to the variables errors warning, statement which
+# respectively contain the number of errors / warnings messages and the total
+# number of statements analyzed. This is used by the global evaluation report
+# (RP0004).
+evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / statement) * 10)
+
+# Add a comment according to your evaluation note. This is used by the global
+# evaluation report (RP0004).
+comment=no
+
+# Display symbolic names of messages in reports
+symbols=yes
+
+
+[TYPECHECK]
+
+# Tells whether missing members accessed in mixin class should be ignored. A
+# mixin class is detected if its name ends with "mixin" (case insensitive).
+ignore-mixin-members=yes
+
+# List of classes names for which member attributes should not be checked
+# (useful for classes with attributes dynamically set).
+ignored-classes=SQLObject
+
+# When zope mode is activated, add a predefined set of Zope acquired attributes
+# to generated-members.
+zope=no
+
+# List of members which are set dynamically and missed by pylint inference
+# system, and so shouldn't trigger E0201 when accessed. Python regular
+# expressions are accepted.
+generated-members=
+    REQUEST,
+    acl_users,
+    aq_parent,
+    objects,
+    DoesNotExist,
+    can_read,
+    can_write,
+    get_url,
+    size,
+    content,
+    status_code,
+# For factory_boy factories
+    create,
+    build,
+# For xblocks
+    fields,
+# For locations
+    tag,
+    org,
+    course,
+    category,
+    name,
+    revision,
+
+[BASIC]
+
+# Required attributes for module, separated by a comma
+required-attributes=
+
+# List of builtins function names that should not be used, separated by a comma
+bad-functions=map,filter,apply,input
+
+# Regular expression which should only match correct module names
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+))$
+
+# Regular expression which should only match correct module level names
+const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__)|log|urlpatterns)$
+
+# Regular expression which should only match correct class names
+class-rgx=[A-Z_][a-zA-Z0-9]+$
+
+# Regular expression which should only match correct function names
+function-rgx=([a-z_][a-z0-9_]{2,30}|test_[a-z0-9_]+)$
+
+# Regular expression which should only match correct method names
+method-rgx=([a-z_][a-z0-9_]{2,60}|setUp|set[Uu]pClass|tearDown|tear[Dd]ownClass|assert[A-Z]\w*|maxDiff|test_[a-z0-9_]+)$
+
+# Regular expression which should only match correct instance attribute names
+attr-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct argument names
+argument-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct variable names
+variable-rgx=[a-z_][a-z0-9_]{2,30}$
+
+# Regular expression which should only match correct list comprehension /
+# generator expression variable names
+inlinevar-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Good variable names which should always be accepted, separated by a comma
+good-names=f,i,j,k,db,ex,Run,_,__
+
+# Bad variable names which should always be refused, separated by a comma
+bad-names=foo,bar,baz,toto,tutu,tata
+
+# Regular expression which should only match functions or classes name which do
+# not require a docstring
+no-docstring-rgx=__.*__|test_.*|setUp|tearDown
+
+
+[MISCELLANEOUS]
+
+# List of note tags to take in consideration, separated by a comma.
+notes=FIXME,XXX,TODO
+
+
+[FORMAT]
+
+# Maximum number of characters on a single line.
+max-line-length=120
+
+# Maximum number of lines in a module
+max-module-lines=1000
+
+# String used as indentation unit. This is usually " " (4 spaces) or "\t" (1
+# tab).
+indent-string='    '
+
+
+[SIMILARITIES]
+
+# Minimum lines number of a similarity.
+min-similarity-lines=4
+
+# Ignore comments when computing similarities.
+ignore-comments=yes
+
+# Ignore docstrings when computing similarities.
+ignore-docstrings=yes
+
+
+[VARIABLES]
+
+# Tells whether we should check for unused import in __init__ files.
+init-import=no
+
+# A regular expression matching the beginning of the name of dummy variables
+# (i.e. not used).
+dummy-variables-rgx=_|dummy|unused|.*_unused
+
+# List of additional names supposed to be defined in builtins. Remember that
+# you should avoid to define new builtins when possible.
+additional-builtins=
+
+
+[IMPORTS]
+
+# Deprecated modules which should not be used, separated by a comma
+deprecated-modules=regsub,string,TERMIOS,Bastion,rexec
+
+# Create a graph of every (i.e. internal and external) dependencies in the
+# given file (report RP0402 must not be disabled)
+import-graph=
+
+# Create a graph of external dependencies in the given file (report RP0402 must
+# not be disabled)
+ext-import-graph=
+
+# Create a graph of internal dependencies in the given file (report RP0402 must
+# not be disabled)
+int-import-graph=
+
+
+[DESIGN]
+
+# Maximum number of arguments for function / method
+max-args=5
+
+# Argument names that match this expression will be ignored. Default to name
+# with leading underscore
+ignored-argument-names=_.*
+
+# Maximum number of locals for function / method body
+max-locals=15
+
+# Maximum number of return / yield for function / method body
+max-returns=6
+
+# Maximum number of branch for function / method body
+max-branchs=12
+
+# Maximum number of statements in function / method body
+max-statements=50
+
+# Maximum number of parents for a class (see R0901).
+max-parents=7
+
+# Maximum number of attributes for a class (see R0902).
+max-attributes=7
+
+# Minimum number of public methods for a class (see R0903).
+min-public-methods=2
+
+# Maximum number of public methods for a class (see R0904).
+max-public-methods=20
+
+
+[CLASSES]
+
+# List of interface methods to ignore, separated by a comma. This is used for
+# instance to not check methods defines in Zope's Interface base class.
+ignore-iface-methods=isImplementedBy,deferred,extends,names,namesAndDescriptions,queryDescriptionFor,getBases,getDescriptionFor,getDoc,getName,getTaggedValue,getTaggedValueTags,isEqualOrExtendedBy,setTaggedValue,isImplementedByInstancesOf,adaptWith,is_implemented_by
+
+# List of method names used to declare (i.e. assign) instance attributes.
+defining-attr-methods=__init__,__new__,setUp
+
+# List of valid names for the first argument in a class method.
+valid-classmethod-first-arg=cls
+
+
+[EXCEPTIONS]
+
+# Exceptions that will emit a warning when being caught. Defaults to
+# "Exception"
+overgeneral-exceptions=Exception

test-requirements.txt

@@ -2,3 +2,4 @@ django-nose==1.2
 coverage==3.7.1
 mock==1.0.1
 django-debug-toolbar==1.2.1
+pylint==1.3.0