Upgrade pylint-1.7.1 and ensure __init__.py file is present. (oppia#3385

) * Upgrade pylint-1.7.1 and ensure __init__.py file is present. * Fix lint issues introduced by new version of pylint.
saketringsia · May 3, 2017 · 6baf9a9 · 6baf9a9
1 parent 56b624d
commit 6baf9a9
Show file tree

Hide file tree

Showing 14 changed files with 20 additions and 68 deletions.
diff --git a/.pylintrc b/.pylintrc
@@ -65,7 +65,7 @@ ignore-imports=yes
 #    too-many-statements
 # and fix those issues.
 
-disable=locally-disabled,locally-enabled,logging-not-lazy,abstract-method,arguments-differ,broad-except,duplicate-code,fixme,missing-docstring,no-member,no-self-use,redefined-variable-type,too-many-arguments,too-many-boolean-expressions,too-many-branches,too-many-instance-attributes,too-many-lines,too-many-locals,too-many-public-methods,too-many-statements
+disable=consider-using-ternary,locally-disabled,locally-enabled,logging-not-lazy,abstract-method,arguments-differ,broad-except,duplicate-code,fixme,len-as-condition,missing-docstring,no-else-return,no-member,no-self-use,not-context-manager,redefined-variable-type,too-many-arguments,too-many-boolean-expressions,too-many-branches,too-many-instance-attributes,too-many-lines,too-many-locals,too-many-public-methods,too-many-nested-blocks,too-many-statements
 
 [REPORTS]
 

diff --git a/core/controllers/base_test.py b/core/controllers/base_test.py
@@ -394,8 +394,7 @@ def test_keys_in_source_code_match_en(self):
         files_checked = 0
         missing_keys_count = 0
         for directory in dirs_to_search:
-            for root, _, files in os.walk(os.path.join(
-                    os.getcwd(), directory)):
+            for root, _, files in os.walk(os.path.join(os.getcwd(), directory)):
                 for filename in files:
                     if filename.endswith('.html'):
                         files_checked += 1

diff --git a/core/domain/exp_jobs_one_off_test.py b/core/domain/exp_jobs_one_off_test.py
@@ -244,9 +244,6 @@ class OneOffExplorationFirstPublishedJobTest(test_utils.GenericTestBase):
 
     EXP_ID = 'exp_id'
 
-    def setUp(self):
-        super(OneOffExplorationFirstPublishedJobTest, self).setUp()
-
     def test_first_published_time_of_exploration_that_is_unpublished(self):
         """This tests that, if an exploration is published, unpublished, and
         then published again, the job uses the first publication time as the
@@ -301,9 +298,6 @@ class ExpSummariesContributorsOneOffJobTest(test_utils.GenericTestBase):
     EMAIL_A = 'emaila@example.com'
     EMAIL_B = 'emailb@example.com'
 
-    def setUp(self):
-        super(ExpSummariesContributorsOneOffJobTest, self).setUp()
-
     def test_contributors_for_valid_contribution(self):
         """Test that if only one commit is made, that the contributor
         list consists of that contributor's user id.

diff --git a/core/domain/exp_services_test.py b/core/domain/exp_services_test.py
@@ -2330,7 +2330,7 @@ def test_get_non_private_exploration_summaries(self):
                         'contributor_ids', 'version',
                         'exploration_model_created_on',
                         'exploration_model_last_updated']
-        for exp_id in actual_summaries.keys():
+        for exp_id in actual_summaries:
             for prop in simple_props:
                 self.assertEqual(getattr(actual_summaries[exp_id], prop),
                                  getattr(expected_summaries[exp_id], prop))
@@ -2372,7 +2372,7 @@ def test_get_all_exploration_summaries(self):
                         'editor_ids', 'viewer_ids', 'contributor_ids',
                         'version', 'exploration_model_created_on',
                         'exploration_model_last_updated']
-        for exp_id in actual_summaries.keys():
+        for exp_id in actual_summaries:
             for prop in simple_props:
                 self.assertEqual(getattr(actual_summaries[exp_id], prop),
                                  getattr(expected_summaries[exp_id], prop))

diff --git a/core/domain/feedback_domain_test.py b/core/domain/feedback_domain_test.py
@@ -103,9 +103,6 @@ def test_to_dict(self):
 class FeedbackAnalyticsDomainUnitTests(test_utils.GenericTestBase):
     EXP_ID = 'exp0'
 
-    def setUp(self):
-        super(FeedbackAnalyticsDomainUnitTests, self).setUp()
-
     def test_to_dict(self):
         expected_thread_analytics = feedback_domain.FeedbackAnalytics(
             self.EXP_ID, 1, 2)

diff --git a/core/domain/value_generators_domain.py b/core/domain/value_generators_domain.py
@@ -88,7 +88,7 @@ def _refresh_registry(cls):
             if name.endswith('_test'):
                 continue
             module = loader.find_module(name).load_module(name)
-            for name, clazz in inspect.getmembers(module, inspect.isclass):
+            for _, clazz in inspect.getmembers(module, inspect.isclass):
                 if issubclass(clazz, BaseValueGenerator):
                     if clazz.__name__ in cls.value_generators_dict:
                         raise Exception(

diff --git a/core/platform/search/gae_search_services.py b/core/platform/search/gae_search_services.py
@@ -126,8 +126,7 @@ def _make_fields(key, value):
     if isinstance(value, numbers.Number):
         return [gae_search.NumberField(name=key, value=value)]
 
-    if isinstance(value, datetime.datetime) or isinstance(
-            value, datetime.date):
+    if isinstance(value, (datetime.datetime, datetime.date)):
         return [gae_search.DateField(name=key, value=value)]
 
     raise ValueError(
@@ -141,10 +140,8 @@ def _validate_list(key, value):
     passed in to make better error messages."""
 
     for ind, element in enumerate(value):
-        if not (isinstance(element, basestring) or
-                isinstance(element, datetime.date) or
-                isinstance(element, datetime.datetime) or
-                isinstance(element, numbers.Number)):
+        if not isinstance(element, (
+                basestring, datetime.date, datetime.datetime, numbers.Number)):
             raise ValueError(
                 'All values of a multi-valued field must be numbers, strings, '
                 'date or datetime instances, The %dth value for field %s has'

diff --git a/core/storage/base_model/gae_models.py b/core/storage/base_model/gae_models.py
@@ -88,10 +88,6 @@ def get(cls, entity_id, strict=True):
                 (cls.__name__, entity_id))
         return entity
 
-    def put(self):
-        """Stores this entity's data."""
-        super(BaseModel, self).put()
-
     @classmethod
     def get_multi(cls, entity_ids, include_deleted=False):
         """Gets list of entities by list of ids.

diff --git a/core/storage/collection/gae_models.py b/core/storage/collection/gae_models.py
@@ -73,23 +73,6 @@ def get_collection_count(cls):
         """Returns the total number of collections."""
         return cls.get_all().count()
 
-    def commit(self, committer_id, commit_message, commit_cmds):
-        """Updates the collection model by applying the given commit_cmds, then
-        saves it.
-
-        Args:
-            committer_id: str. The user_id of the user who committed the
-                change.
-            commit_message: str. The commit description message.
-            commit_cmds: list(dict). A list of commands, describing changes
-                made in this model, which should give sufficient information to
-                reconstruct the commit. Each dict always contains:
-                    cmd: str. Unique command.
-                and additional arguments for that command.
-        """
-        super(CollectionModel, self).commit(
-            committer_id, commit_message, commit_cmds)
-
     def _trusted_commit(
             self, committer_id, commit_type, commit_message, commit_cmds):
         """Record the event to the commit log after the model commit.

diff --git a/core/storage/exploration/gae_models.py b/core/storage/exploration/gae_models.py
@@ -95,22 +95,6 @@ def get_exploration_count(cls):
         """Returns the total number of explorations."""
         return cls.get_all().count()
 
-    def commit(self, committer_id, commit_message, commit_cmds):
-        """Updates the exploration using the properties dict, then saves it.
-
-        Args:
-            committer_id: str. The user id of the user who committed the
-                change.
-            commit_message: str. The commit description message.
-            commit_cmds: list(dict). A list of commands, describing changes
-                made in this model, which should give sufficient information to
-                reconstruct the commit. Each dict always contains:
-                    cmd: str. Unique command.
-                and additional arguments for that command.
-        """
-        super(ExplorationModel, self).commit(
-            committer_id, commit_message, commit_cmds)
-
     def _trusted_commit(
             self, committer_id, commit_type, commit_message, commit_cmds):
         """Record the event to the commit log after the model commit.

diff --git a/extensions/interactions/base.py b/extensions/interactions/base.py
@@ -265,8 +265,8 @@ def get_rule_param_type(self, rule_name, rule_param_name):
         """Gets the parameter type for a given rule parameter name."""
         rule_param_list = self.get_rule_param_list(rule_name)
 
-        for rule_name, rule_type in rule_param_list:
-            if rule_name == rule_param_name:
-                return rule_type
+        for param_name, param_type in rule_param_list:
+            if param_name == rule_param_name:
+                return param_type
         raise Exception(
             'Rule %s has no param called %s' % (rule_name, rule_param_name))
diff --git a/main.py b/main.py
@@ -76,10 +76,9 @@ def get(self):
                 self.user_id)
 
             # 'Creator' is a user who has created or edited an exploration.
-            user_is_creator = (
-                user_contributions is not None and
-                (len(user_contributions.created_exploration_ids) > 0 or
-                 len(user_contributions.edited_exploration_ids) > 0))
+            user_is_creator = user_contributions and (
+                user_contributions.created_exploration_ids or
+                user_contributions.edited_exploration_ids)
             if user_is_creator:
                 self.redirect(feconf.DASHBOARD_URL)
             else:

diff --git a/scripts/install_third_party.sh b/scripts/install_third_party.sh
@@ -107,10 +107,13 @@ if [ ! -d "$TOOLS_DIR/numpy-1.6.1" ]; then
 fi
 
 echo Checking if pylint is installed in $TOOLS_DIR/pip_packages
-if [ ! -d "$TOOLS_DIR/pylint-1.5.2" ]; then
+if [ ! -d "$TOOLS_DIR/pylint-1.7.1" ]; then
   echo Installing Pylint
 
-  pip install pylint==1.5.2 --target="$TOOLS_DIR/pylint-1.5.2"
+  pip install pylint==1.7.1 --target="$TOOLS_DIR/pylint-1.7.1"
+  # Add __init__.py file so that pylint dependency backports are resolved
+  # correctly.
+  touch $TOOLS_DIR/pylint-1.7.1/backports/__init__.py
 fi
 
 # Install webtest.

diff --git a/scripts/pre_commit_linter.py b/scripts/pre_commit_linter.py
@@ -129,7 +129,7 @@
     print 'ERROR    Please run this script from the oppia root directory.'
 
 _PARENT_DIR = os.path.abspath(os.path.join(os.getcwd(), os.pardir))
-_PYLINT_PATH = os.path.join(_PARENT_DIR, 'oppia_tools', 'pylint-1.5.2')
+_PYLINT_PATH = os.path.join(_PARENT_DIR, 'oppia_tools', 'pylint-1.7.1')
 if not os.path.exists(_PYLINT_PATH):
     print ''
     print 'ERROR    Please run start.sh first to install pylint '