Enforce variables to be in snake case (oppia#5943)

* Variables should be in snake case * Changed variable names * Resolved conflicts
quantumferret · Dec 22, 2018 · 238f8d5 · 238f8d5
1 parent a5aaaf6
commit 238f8d5
Show file tree

Hide file tree

Showing 15 changed files with 94 additions and 83 deletions.
diff --git a/.pylintrc b/.pylintrc
@@ -32,7 +32,16 @@ good-names=e,_,d,f,i,l,p,w,fn,fs,id,pc,sc,zf,setUp,tearDown,longMessage,maxDiff
 dummy-variables-rgx=_|unused_*
 
 # Regular expression which should match good variable names
-variable-rgx=[a-z_]+
+variable-rgx=[a-z_][a-z0-9_]*$
+
+# Regular expression matching correct attribute names
+attr-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Regular expression matching correct argument names
+argument-rgx=[A-Za-z_][A-Za-z0-9_]*$
+
+# Regular expression matching correct module names
+module-rgx=[A-Za-z_][A-Za-z0-9_]*$
 
 # Regular expression which should only match function or class names that do
 # not require a docstring.
@@ -58,7 +67,7 @@ ignore-imports=yes
 
 [MESSAGES CONTROL]
 
-disable=consider-using-ternary,locally-disabled,locally-enabled,logging-not-lazy,abstract-method,arguments-differ,broad-except,duplicate-code,fixme,inconsistent-return-statements,invalid-name,len-as-condition,missing-docstring,missing-raises-doc,multiple-constructor-doc,no-else-return,no-member,no-self-use,not-context-manager,redefined-variable-type,redundant-returns-doc,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,wrong-import-order
+disable=consider-using-ternary,locally-disabled,locally-enabled,logging-not-lazy,abstract-method,arguments-differ,broad-except,duplicate-code,fixme,inconsistent-return-statements,len-as-condition,missing-docstring,missing-raises-doc,multiple-constructor-doc,no-else-return,no-member,no-self-use,not-context-manager,redefined-variable-type,redundant-returns-doc,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,wrong-import-order
 
 [REPORTS]
 

diff --git a/core/controllers/reader_test.py b/core/controllers/reader_test.py
@@ -210,9 +210,9 @@ class ExplorationPretestsUnitTest(test_utils.GenericTestBase):
 
     def test_get_exploration_pretests(self):
         super(ExplorationPretestsUnitTest, self).setUp()
-        STORY_ID = story_services.get_new_story_id()
+        story_id = story_services.get_new_story_id()
         self.save_new_story(
-            STORY_ID, 'user', 'Title', 'Description', 'Notes'
+            story_id, 'user', 'Title', 'Description', 'Notes'
         )
         changelist = [
             story_domain.StoryChange({
@@ -221,10 +221,10 @@ def test_get_exploration_pretests(self):
                 'title': 'Title 1'
             })
         ]
-        story_services.update_story('user', STORY_ID, changelist, 'Added node.')
-        SKILL_ID = skill_services.get_new_skill_id()
+        story_services.update_story('user', story_id, changelist, 'Added node.')
+        skill_id = skill_services.get_new_skill_id()
         self.save_new_skill(
-            SKILL_ID, 'user', 'Description')
+            skill_id, 'user', 'Description')
 
         exp_id = '0'
         exp_id_2 = '1'
@@ -237,7 +237,7 @@ def test_get_exploration_pretests(self):
             'property_name': (
                 story_domain.STORY_NODE_PROPERTY_PREREQUISITE_SKILL_IDS),
             'old_value': [],
-            'new_value': [SKILL_ID],
+            'new_value': [skill_id],
             'node_id': 'node_1'
         }), story_domain.StoryChange({
             'cmd': story_domain.CMD_UPDATE_STORY_NODE_PROPERTY,
@@ -248,7 +248,7 @@ def test_get_exploration_pretests(self):
             'node_id': 'node_1'
         })]
         story_services.update_story(
-            'user', STORY_ID, change_list, 'Updated Node 1.')
+            'user', story_id, change_list, 'Updated Node 1.')
         question_id = question_services.get_new_question_id()
         self.save_new_question(
             question_id, 'user',
@@ -258,20 +258,20 @@ def test_get_exploration_pretests(self):
             question_id_2, 'user',
             self._create_valid_question_data('ABC'))
         question_services.create_new_question_skill_link(
-            question_id, SKILL_ID)
+            question_id, skill_id)
         question_services.create_new_question_skill_link(
-            question_id_2, SKILL_ID)
+            question_id_2, skill_id)
         # Call the handler.
         with self.swap(feconf, 'NUM_PRETEST_QUESTIONS', 1):
             json_response_1 = self.get_json(
                 '%s/%s?story_id=%s&cursor=' % (
-                    feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id, STORY_ID))
+                    feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id, story_id))
             next_cursor = json_response_1['next_start_cursor']
 
             self.assertEqual(len(json_response_1['pretest_question_dicts']), 1)
             json_response_2 = self.get_json(
                 '%s/%s?story_id=%s&cursor=%s' % (
-                    feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id, STORY_ID,
+                    feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id, story_id,
                     next_cursor))
             self.assertEqual(len(json_response_2['pretest_question_dicts']), 1)
             self.assertNotEqual(
@@ -280,7 +280,7 @@ def test_get_exploration_pretests(self):
 
         response = self.testapp.get(
             '%s/%s?story_id=%s' % (
-                feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id_2, STORY_ID),
+                feconf.EXPLORATION_PRETESTS_URL_PREFIX, exp_id_2, story_id),
             expect_errors=True)
         self.assertEqual(response.status_int, 400)
 

diff --git a/core/domain/exp_jobs_one_off.py b/core/domain/exp_jobs_one_off.py
@@ -404,7 +404,7 @@ def map(item):
 
         # Commit commands which are required to generate state id mapping for
         # given version of exploration from previous version of exploration.
-        RELEVANT_COMMIT_CMDS = [
+        relevant_commit_cmds = [
             exp_domain.CMD_ADD_STATE,
             exp_domain.CMD_RENAME_STATE,
             exp_domain.CMD_DELETE_STATE,
@@ -453,7 +453,7 @@ def map(item):
             change_list = [
                 exp_domain.ExplorationChange(change_cmd)
                 for change_cmd in snapshot['commit_cmds']
-                if change_cmd['cmd'] in RELEVANT_COMMIT_CMDS
+                if change_cmd['cmd'] in relevant_commit_cmds
             ]
 
             try:

diff --git a/core/domain/html_validation_service.py b/core/domain/html_validation_service.py
@@ -39,15 +39,15 @@ def escape_html(unescaped_html_data):
         str. Escaped HTML string.
     """
     # Replace list to escape html strings.
-    REPLACE_LIST_FOR_ESCAPING = [
+    replace_list_for_escaping = [
         ('&', '&amp;'),
         ('"', '&quot;'),
         ('\'', '&#39;'),
         ('<', '&lt;'),
         ('>', '&gt;')
     ]
     escaped_html_data = unescaped_html_data
-    for replace_tuple in REPLACE_LIST_FOR_ESCAPING:
+    for replace_tuple in replace_list_for_escaping:
         escaped_html_data = escaped_html_data.replace(
             replace_tuple[0], replace_tuple[1])
 
@@ -64,15 +64,15 @@ def unescape_html(escaped_html_data):
         str. Unescaped HTML string.
     """
     # Replace list to unescape html strings.
-    REPLACE_LIST_FOR_UNESCAPING = [
+    replace_list_for_unescaping = [
         ('&quot;', '"'),
         ('&#39;', '\''),
         ('&lt;', '<'),
         ('&gt;', '>'),
         ('&amp;', '&')
     ]
     unescaped_html_data = escaped_html_data
-    for replace_tuple in REPLACE_LIST_FOR_UNESCAPING:
+    for replace_tuple in replace_list_for_unescaping:
         unescaped_html_data = unescaped_html_data.replace(
             replace_tuple[0], replace_tuple[1])
 
@@ -387,11 +387,11 @@ def convert_to_ckeditor(html_data):
         while li.parent.name in ['li', 'p']:
             li.parent.unwrap()
 
-    LIST_TAGS = ['ol', 'ul']
+    list_tags = ['ol', 'ul']
 
     # Ensure li is wrapped in ol/ul.
     for li in soup.findAll(name='li'):
-        if li.parent.name not in LIST_TAGS:
+        if li.parent.name not in list_tags:
             new_parent = soup.new_tag('ul')
             next_sib = list(li.next_siblings)
             li.wrap(new_parent)
@@ -402,7 +402,7 @@ def convert_to_ckeditor(html_data):
                     break
 
     # Ensure that the children of ol/ul are li/pre.
-    for tag_name in LIST_TAGS:
+    for tag_name in list_tags:
         for tag in soup.findAll(name=tag_name):
             for child in tag.children:
                 if child.name not in ['li', 'pre', 'ol', 'ul']:
@@ -420,7 +420,7 @@ def convert_to_ckeditor(html_data):
     for p in soup.findAll(name='p'):
         if p.parent.name == 'pre':
             p.unwrap()
-        elif p.parent.name in LIST_TAGS:
+        elif p.parent.name in list_tags:
             p.wrap(soup.new_tag('li'))
 
     # This block ensures that ol/ul tag is not a direct child of another ul/ol
@@ -430,9 +430,9 @@ def convert_to_ckeditor(html_data):
     # i.e. if any ol/ul has parent as ol/ul and a previous sibling as li
     # it is wrapped in its previous sibling. If there is no previous sibling,
     # the tag is unwrapped.
-    for tag_name in LIST_TAGS:
+    for tag_name in list_tags:
         for tag in soup.findAll(name=tag_name):
-            if tag.parent.name in LIST_TAGS:
+            if tag.parent.name in list_tags:
                 prev_sib = tag.previous_sibling
                 if prev_sib and prev_sib.name == 'li':
                     prev_sib.append(tag)
@@ -612,12 +612,12 @@ def _validate_soup_for_rte(soup, rte_format, err_dict):
         bool. Boolean indicating whether a html string is valid for given RTE.
     """
     if rte_format == feconf.RTE_FORMAT_TEXTANGULAR:
-        RTE_TYPE = 'RTE_TYPE_TEXTANGULAR'
+        rte_type = 'RTE_TYPE_TEXTANGULAR'
     else:
-        RTE_TYPE = 'RTE_TYPE_CKEDITOR'
+        rte_type = 'RTE_TYPE_CKEDITOR'
     allowed_parent_list = feconf.RTE_CONTENT_SPEC[
-        RTE_TYPE]['ALLOWED_PARENT_LIST']
-    allowed_tag_list = feconf.RTE_CONTENT_SPEC[RTE_TYPE]['ALLOWED_TAG_LIST']
+        rte_type]['ALLOWED_PARENT_LIST']
+    allowed_tag_list = feconf.RTE_CONTENT_SPEC[rte_type]['ALLOWED_TAG_LIST']
 
     is_invalid = False
 
@@ -684,15 +684,15 @@ def validate_customization_args(html_list):
     # Dictionary to hold html strings in which customization arguments
     # are invalid.
     err_dict = {}
-    RICH_TEXT_COMPONENT_TAG_NAMES = (
+    rich_text_component_tag_names = (
         INLINE_COMPONENT_TAG_NAMES + BLOCK_COMPONENT_TAG_NAMES)
 
     tags_to_original_html_strings = {}
     for html_string in html_list:
         soup = bs4.BeautifulSoup(
             html_string.encode(encoding='utf-8'), 'html.parser')
 
-        for tag_name in RICH_TEXT_COMPONENT_TAG_NAMES:
+        for tag_name in rich_text_component_tag_names:
             for tag in soup.findAll(name=tag_name):
                 tags_to_original_html_strings[tag] = html_string
 
@@ -719,8 +719,8 @@ def _validate_customization_args_in_tag(tag):
         Error message if the attributes of tag are invalid.
     """
 
-    COMPONENT_TYPES_TO_COMPONENT_CLASSES = rte_component_registry.Registry.get_component_types_to_component_classes() # pylint: disable=line-too-long
-    SIMPLE_COMPONENT_TAG_NAMES = (
+    component_types_to_component_classes = rte_component_registry.Registry.get_component_types_to_component_classes() # pylint: disable=line-too-long
+    simple_component_tag_names = (
         rte_component_registry.Registry.get_simple_component_tag_names())
     tag_name = tag.name
     value_dict = {}
@@ -730,12 +730,12 @@ def _validate_customization_args_in_tag(tag):
         value_dict[attr] = json.loads(unescape_html(attrs[attr]))
 
     try:
-        COMPONENT_TYPES_TO_COMPONENT_CLASSES[tag_name].validate(value_dict)
+        component_types_to_component_classes[tag_name].validate(value_dict)
         if tag_name == 'oppia-noninteractive-collapsible':
             content_html = value_dict['content-with-value']
             soup_for_collapsible = bs4.BeautifulSoup(
                 content_html, 'html.parser')
-            for component_name in SIMPLE_COMPONENT_TAG_NAMES:
+            for component_name in simple_component_tag_names:
                 for component_tag in soup_for_collapsible.findAll(
                         name=component_name):
                     for err_msg in _validate_customization_args_in_tag(
@@ -748,7 +748,7 @@ def _validate_customization_args_in_tag(tag):
                 content_html = tab_content['content']
                 soup_for_tabs = bs4.BeautifulSoup(
                     content_html, 'html.parser')
-                for component_name in SIMPLE_COMPONENT_TAG_NAMES:
+                for component_name in simple_component_tag_names:
                     for component_tag in soup_for_tabs.findAll(name=tag_name):
                         for err_msg in _validate_customization_args_in_tag(
                                 component_tag):

diff --git a/core/domain/html_validation_service_test.py b/core/domain/html_validation_service_test.py
@@ -1034,21 +1034,21 @@ def test_add_dimensions_to_image_tags(self):
             )
         }]
 
-        EXP_ID = 'eid'
-        OWNER_ID = 'Admin'
+        exp_id = 'eid'
+        owner_id = 'Admin'
 
         with open(os.path.join(feconf.TESTS_DATA_DIR, 'img.png')) as f:
             raw_image = f.read()
         fs = fs_domain.AbstractFileSystem(
-            fs_domain.ExplorationFileSystem('exploration/%s' % EXP_ID))
-        fs.commit(OWNER_ID, 'image/abc1.png', raw_image, mimetype='image/png')
-        fs.commit(OWNER_ID, 'image/abc2.png', raw_image, mimetype='image/png')
-        fs.commit(OWNER_ID, 'image/abc3.png', raw_image, mimetype='image/png')
+            fs_domain.ExplorationFileSystem('exploration/%s' % exp_id))
+        fs.commit(owner_id, 'image/abc1.png', raw_image, mimetype='image/png')
+        fs.commit(owner_id, 'image/abc2.png', raw_image, mimetype='image/png')
+        fs.commit(owner_id, 'image/abc3.png', raw_image, mimetype='image/png')
 
         for test_case in test_cases:
             self.assertEqual(
                 html_validation_service.add_dimensions_to_image_tags(
-                    EXP_ID, test_case['html_content']),
+                    exp_id, test_case['html_content']),
                 test_case['expected_output'])
 
     def test_add_dimensions_to_image_tags_when_no_filepath_specified(self):
@@ -1097,20 +1097,20 @@ def test_add_dimensions_to_image_tags_when_no_filepath_specified(self):
             )
         }]
 
-        EXP_ID = 'eid'
-        OWNER_ID = 'Admin'
+        exp_id = 'eid'
+        owner_id = 'Admin'
 
         with open(os.path.join(feconf.TESTS_DATA_DIR, 'img.png')) as f:
             raw_image = f.read()
         fs = fs_domain.AbstractFileSystem(
-            fs_domain.ExplorationFileSystem('exploration/%s' % EXP_ID))
-        fs.commit(OWNER_ID, 'image/img.png', raw_image, mimetype='image/png')
-        fs.commit(OWNER_ID, 'image/abc3.png', raw_image, mimetype='image/png')
+            fs_domain.ExplorationFileSystem('exploration/%s' % exp_id))
+        fs.commit(owner_id, 'image/img.png', raw_image, mimetype='image/png')
+        fs.commit(owner_id, 'image/abc3.png', raw_image, mimetype='image/png')
 
         for test_case in test_cases:
             self.assertEqual(
                 html_validation_service.add_dimensions_to_image_tags(
-                    EXP_ID, test_case['html_content']),
+                    exp_id, test_case['html_content']),
                 test_case['expected_output'])
 
     def test_regenerate_image_filename_using_dimensions(self):

diff --git a/core/domain/question_domain.py b/core/domain/question_domain.py
@@ -223,7 +223,7 @@ def partial_validate(self):
             raise utils.ValidationError(
                 'Invalid language code: %s' % self.language_code)
 
-        INTERACTION_SPECS = interaction_registry.Registry.get_all_specs()
+        interaction_specs = interaction_registry.Registry.get_all_specs()
         at_least_one_correct_answer = False
         dest_is_specified = False
         interaction = self.question_state_data.interaction
@@ -256,7 +256,7 @@ def partial_validate(self):
 
         if (
                 (interaction.solution is None) and
-                (INTERACTION_SPECS[interaction.id]['can_have_solution'])):
+                (interaction_specs[interaction.id]['can_have_solution'])):
             raise utils.ValidationError(
                 'Expected the question to have a solution'
             )

diff --git a/core/domain/stats_jobs_one_off.py b/core/domain/stats_jobs_one_off.py
@@ -92,7 +92,7 @@ def map(exploration_model):
         if exploration_model.deleted:
             return
 
-        RELEVANT_COMMIT_CMDS = [
+        relevant_commit_cmds = [
             exp_domain.CMD_ADD_STATE,
             exp_domain.CMD_RENAME_STATE,
             exp_domain.CMD_DELETE_STATE,
@@ -140,7 +140,7 @@ def map(exploration_model):
                     change_list = [
                         exp_domain.ExplorationChange(change_dict)
                         for change_dict in change_dicts
-                        if change_dict['cmd'] in RELEVANT_COMMIT_CMDS]
+                        if change_dict['cmd'] in relevant_commit_cmds]
                     exp_versions_diff = exp_domain.ExplorationVersionsDiff(
                         change_list)
 
@@ -523,7 +523,7 @@ def _apply_state_name_changes(cls, prev_stats_dict, change_dicts):
             dict. A dict representation of an ExplorationStatsModel
                 with updated state_stats_mapping and version.
         """
-        RELEVANT_COMMIT_CMDS = [
+        relevant_commit_cmds = [
             exp_domain.CMD_ADD_STATE,
             exp_domain.CMD_RENAME_STATE,
             exp_domain.CMD_DELETE_STATE,
@@ -533,7 +533,7 @@ def _apply_state_name_changes(cls, prev_stats_dict, change_dicts):
         change_list = [
             exp_domain.ExplorationChange(change_dict)
             for change_dict in change_dicts
-            if change_dict['cmd'] in RELEVANT_COMMIT_CMDS]
+            if change_dict['cmd'] in relevant_commit_cmds]
         exp_versions_diff = exp_domain.ExplorationVersionsDiff(change_list)
 
         # Handling state deletions, renames and additions (in that order). The