Fix part of #3905: Add check for period at comment and docstring end (#…

…4759)

* add check for period at comment and docstring end

* modify test

* fix errors

* address review comments

* fix minor issue in test

* use strict punctuations

* fix docstring check

* fix comments

* add test for single line docstring spanning two lines

* improve messages

* add brackets to punctuation list

* address review comments

* fix brackets

* fix testing

* fix if statement style

constants.py

@@ -32,7 +32,7 @@ def parse_json_from_js(js_file):
 
 
 class Constants(dict):
-    """Transforms dict to object, attributes can be accesed by dot notation"""
+    """Transforms dict to object, attributes can be accesed by dot notation."""
     __getattr__ = dict.__getitem__
 
 

core/controllers/base.py

@@ -127,7 +127,7 @@ class BaseHandler(webapp2.RequestHandler):
     # users have agreed to the latest terms.
     REDIRECT_UNFINISHED_SIGNUPS = True
 
-    # What format the get method returns when exception raised, json or html
+    # What format the get method returns when exception raised, json or html.
     GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_HTML
 
     @webapp2.cached_property

core/controllers/base_test.py

@@ -137,7 +137,7 @@ def test_root_redirect_rules_for_logged_in_learners(self):
     def test_root_redirect_rules_for_users_with_no_user_contribution_model(
             self):
         self.login(self.TEST_LEARNER_EMAIL)
-        # delete the UserContributionModel
+        # delete the UserContributionModel.
         user_id = user_services.get_user_id_from_username(
             self.TEST_LEARNER_USERNAME)
         user_contribution_model = user_models.UserContributionsModel.get(

core/controllers/classifier.py

@@ -133,7 +133,7 @@ class NextJobHandler(base.BaseHandler):
 
     @acl_decorators.open_access
     def post(self):
-        """Handles POST requests. """
+        """Handles POST requests."""
         signature = self.payload.get('signature')
         vm_id = self.payload.get('vm_id')
         message = self.payload.get('message')

core/controllers/classifier_test.py

@@ -13,7 +13,8 @@
 # limitations under the License.
 
 """Tests for the controllers that communicate with VM for training
-classifiers."""
+classifiers.
+"""
 
 import json
 import os

core/controllers/collection_editor.py

@@ -76,7 +76,8 @@ class EditableCollectionDataHandler(CollectionEditorHandler):
     """A data handler for collections which supports writing."""
 
     def _require_valid_version(self, version_from_payload, collection_version):
-        """Check that the payload version matches the given collection version.
+        """Check that the payload version matches the given collection
+        version.
         """
         if version_from_payload is None:
             raise base.BaseHandler.InvalidInputException(
@@ -93,7 +94,7 @@ def get(self, collection_id):
         """Populates the data on the individual collection page."""
 
         try:
-            # Try to retrieve collection
+            # Try to retrieve collection.
             collection_dict = (
                 summary_services.get_learner_collection_dict_by_id(
                     collection_id, self.user,

core/controllers/collection_editor_test.py

@@ -107,7 +107,7 @@ def test_editable_collection_handler_get(self):
         self.assertEqual(response.status_int, 302)
 
         # Check that whitelisted users can access the data
-        # from the editable_collection_data_handler
+        # from the editable_collection_data_handler.
         self.login(self.EDITOR_EMAIL)
 
         json_response = self.get_json(
@@ -117,7 +117,7 @@ def test_editable_collection_handler_get(self):
         self.logout()
 
     def test_editable_collection_handler_put_cannot_access(self):
-        """Check that non-editors cannot access editable put handler"""
+        """Check that non-editors cannot access editable put handler."""
         whitelisted_usernames = [self.EDITOR_USERNAME, self.VIEWER_USERNAME]
         self.set_collection_editors(whitelisted_usernames)
 
@@ -148,7 +148,7 @@ def test_editable_collection_handler_put_cannot_access(self):
         self.logout()
 
     def test_editable_collection_handler_put_can_access(self):
-        """Check that editors can access put handler"""
+        """Check that editors can access put handler."""
         whitelisted_usernames = [self.EDITOR_USERNAME, self.VIEWER_USERNAME]
         self.set_collection_editors(whitelisted_usernames)
 

core/controllers/editor.py

@@ -611,7 +611,7 @@ def get(self, exploration_id):
         output_format = self.request.get('output_format', default_value='zip')
         width = int(self.request.get('width', default_value=80))
 
-        # If the title of the exploration has changed, we use the new title
+        # If the title of the exploration has changed, we use the new title.
         filename = 'oppia-%s-v%s' % (
             utils.to_ascii(exploration.title.replace(' ', '')), version)
 

core/controllers/editor_test.py

@@ -67,13 +67,15 @@ def setUp(self):
 
     def assert_can_edit(self, response_body):
         """Returns True if the response body indicates that the exploration is
-        editable."""
+        editable.
+        """
         self.assertIn(self.CAN_EDIT_STR, response_body)
         self.assertNotIn(self.CANNOT_EDIT_STR, response_body)
 
     def assert_cannot_edit(self, response_body):
         """Returns True if the response body indicates that the exploration is
-        not editable."""
+        not editable.
+        """
         self.assertIn(self.CANNOT_EDIT_STR, response_body)
         self.assertNotIn(self.CAN_EDIT_STR, response_body)
 
@@ -281,7 +283,7 @@ def _submit_answer(
             with self.swap(
                 jobs_registry, 'ALL_CONTINUOUS_COMPUTATION_MANAGERS',
                 self.ALL_CC_MANAGERS_FOR_TESTS):
-                # Run job on exploration with answers
+                # Run job on exploration with answers.
                 stats_jobs_continuous_test.ModifiedInteractionAnswerSummariesAggregator.start_computation() # pylint: disable=line-too-long
                 self.assertEqual(
                     self.count_jobs_in_taskqueue(
@@ -681,7 +683,7 @@ def test_exploration_download_handler_for_default_exploration(self):
         self.login(self.EDITOR_EMAIL)
         owner_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
 
-        # Create a simple exploration
+        # Create a simple exploration.
         exp_id = 'eid'
         self.save_new_valid_exploration(
             exp_id, owner_id,
@@ -703,12 +705,12 @@ def test_exploration_download_handler_for_default_exploration(self):
             owner_id, exploration, '', [])
         response = self.testapp.get('/create/%s' % exp_id)
 
-        # Check download to zip file
-        # Download to zip file using download handler
+        # Check download to zip file.
+        # Download to zip file using download handler.
         download_url = '/createhandler/download/%s' % exp_id
         response = self.testapp.get(download_url)
 
-        # Check downloaded zip file
+        # Check downloaded zip file.
         self.assertEqual(response.headers['Content-Type'], 'text/plain')
         filename = 'oppia-ThetitleforZIPdownloadhandlertest!-v2.zip'
         self.assertEqual(response.headers['Content-Disposition'],
@@ -718,33 +720,33 @@ def test_exploration_download_handler_for_default_exploration(self):
             zf_saved.namelist(),
             ['The title for ZIP download handler test!.yaml'])
 
-        # Load golden zip file
+        # Load golden zip file.
         with open(os.path.join(
             feconf.TESTS_DATA_DIR,
             'oppia-ThetitleforZIPdownloadhandlertest!-v2-gold.zip'),
                   'rb') as f:
             golden_zipfile = f.read()
         zf_gold = zipfile.ZipFile(StringIO.StringIO(golden_zipfile))
-        # Compare saved with golden file
+        # Compare saved with golden file.
         self.assertEqual(
             zf_saved.open(
                 'The title for ZIP download handler test!.yaml').read(),
             zf_gold.open(
                 'The title for ZIP download handler test!.yaml').read())
 
-        # Check download to JSON
+        # Check download to JSON.
         exploration.update_objective('Test JSON download')
         exp_services._save_exploration(  # pylint: disable=protected-access
             owner_id, exploration, '', [])
 
-        # Download to JSON string using download handler
+        # Download to JSON string using download handler.
         self.maxDiff = None
         download_url = (
             '/createhandler/download/%s?output_format=%s&width=50' %
             (exp_id, feconf.OUTPUT_FORMAT_JSON))
         response = self.get_json(download_url)
 
-        # Check downloaded dict
+        # Check downloaded dict.
         self.assertEqual(self.SAMPLE_JSON_CONTENT, response)
 
         self.logout()
@@ -753,7 +755,7 @@ def test_state_yaml_handler(self):
         self.login(self.EDITOR_EMAIL)
         owner_id = self.get_user_id_from_email(self.EDITOR_EMAIL)
 
-        # Create a simple exploration
+        # Create a simple exploration.
         exp_id = 'eid'
         self.save_new_valid_exploration(
             exp_id, owner_id,
@@ -933,7 +935,7 @@ class VersioningIntegrationTest(BaseEditorControllerTest):
     EXP_ID = '0'
 
     def setUp(self):
-        """Create exploration with two versions"""
+        """Create exploration with two versions."""
         super(VersioningIntegrationTest, self).setUp()
 
         exp_services.load_demo(self.EXP_ID)
@@ -962,27 +964,27 @@ def setUp(self):
 
     def test_reverting_to_old_exploration(self):
         """Test reverting to old exploration versions."""
-        # Open editor page
+        # Open editor page.
         response = self.testapp.get(
             '%s/%s' % (feconf.EDITOR_URL_PREFIX, self.EXP_ID))
         csrf_token = self.get_csrf_token_from_response(response)
 
-        # May not revert to any version that's not 1
+        # May not revert to any version that's not 1.
         for rev_version in (-1, 0, 2, 3, 4, '1', ()):
             response_dict = self.post_json(
                 '/createhandler/revert/%s' % self.EXP_ID, {
                     'current_version': 2,
                     'revert_to_version': rev_version
                 }, csrf_token, expect_errors=True, expected_status_int=400)
 
-            # Check error message
+            # Check error message.
             if not isinstance(rev_version, int):
                 self.assertIn('Expected an integer', response_dict['error'])
             else:
                 self.assertIn('Cannot revert to version',
                               response_dict['error'])
 
-            # Check that exploration is really not reverted to old version
+            # Check that exploration is really not reverted to old version.
             reader_dict = self.get_json(
                 '%s/%s' % (feconf.EXPLORATION_INIT_URL_PREFIX, self.EXP_ID))
             init_state_name = reader_dict['exploration']['init_state_name']
@@ -992,15 +994,15 @@ def test_reverting_to_old_exploration(self):
             self.assertIn('ABC', init_content)
             self.assertNotIn('Hi, welcome to Oppia!', init_content)
 
-        # Revert to version 1
+        # Revert to version 1.
         rev_version = 1
         response_dict = self.post_json(
             '/createhandler/revert/%s' % self.EXP_ID, {
                 'current_version': 2,
                 'revert_to_version': rev_version
             }, csrf_token)
 
-        # Check that exploration is really reverted to version 1
+        # Check that exploration is really reverted to version 1.
         reader_dict = self.get_json(
             '%s/%s' % (feconf.EXPLORATION_INIT_URL_PREFIX, self.EXP_ID))
 
@@ -1077,7 +1079,7 @@ def test_user_banning(self):
         # Ban joe.
         self.set_banned_users(['joe'])
 
-        # Test that Joe is banned. (He can still access the library page.)
+        # Test that Joe is banned (He can still access the library page).
         response = self.testapp.get(
             feconf.LIBRARY_INDEX_URL, expect_errors=True)
         self.assertEqual(response.status_int, 200)
@@ -1118,7 +1120,7 @@ def test_for_assign_role_for_exploration(self):
         self.signup(
             self.COLLABORATOR3_EMAIL, username=self.COLLABORATOR3_USERNAME)
 
-        # Owner creates exploration
+        # Owner creates exploration.
         self.login(self.OWNER_EMAIL)
         exp_id = 'eid'
         self.save_new_valid_exploration(
@@ -1135,7 +1137,7 @@ def test_for_assign_role_for_exploration(self):
             '%s/%s' % (feconf.EDITOR_URL_PREFIX, exp_id))
         csrf_token = self.get_csrf_token_from_response(response)
 
-        # Owner adds rights for other users
+        # Owner adds rights for other users.
         rights_url = '%s/%s' % (feconf.EXPLORATION_RIGHTS_PREFIX, exp_id)
         self.put_json(
             rights_url, {
@@ -1298,7 +1300,7 @@ class UserExplorationEmailsIntegrationTest(BaseEditorControllerTest):
     def test_user_exploration_emails_handler(self):
         """Test user exploration emails handler."""
 
-        # Owner creates exploration
+        # Owner creates exploration.
         self.login(self.OWNER_EMAIL)
         exp_id = 'eid'
         self.save_new_valid_exploration(
@@ -1317,7 +1319,7 @@ def test_user_exploration_emails_handler(self):
         self.assertFalse(exp_email_preferences.mute_feedback_notifications)
         self.assertFalse(exp_email_preferences.mute_suggestion_notifications)
 
-        # Owner changes email preferences
+        # Owner changes email preferences.
         emails_url = '%s/%s' % (feconf.USER_EXPLORATION_EMAILS_PREFIX, exp_id)
         self.put_json(
             emails_url, {
@@ -1656,7 +1658,7 @@ def test_draft_not_updated_validation_error(self):
             '%s.%s' % (self.owner_id, self.EXP_ID2))
         self.assertEqual(
             exp_user_data.draft_change_list, self.DRAFT_CHANGELIST)
-            #id is incremented the first time but not the second
+            #id is incremented the first time but not the second.
         self.assertEqual(exp_user_data.draft_change_list_id, 2)
         self.assertEqual(
             response, {'status_code': 400,

core/controllers/feedback.py

@@ -124,8 +124,8 @@ def get(self):
 
 class FeedbackStatsHandler(base.BaseHandler):
     """Returns Feedback stats for an exploration.
-        - Number of open threads
-        - Number of total threads
+        - Number of open threads.
+        - Number of total threads.
     """
 
     GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
@@ -229,7 +229,8 @@ def get(self, exploration_id):
 class FeedbackThreadViewEventHandler(base.BaseHandler):
     """Records when the given user views a feedback thread, in order to clear
     viewed feedback messages from emails that might be sent in future to this
-    user."""
+    user.
+    """
 
     @acl_decorators.can_comment_on_feedback_thread
     def post(self, exploration_id):

core/controllers/library.py

@@ -149,7 +149,8 @@ def get(self):
 
 
 class LibraryGroupPage(base.BaseHandler):
-    """The page for displaying top rated and recently published explorations.
+    """The page for displaying top rated and recently published
+    explorations.
     """
 
     @acl_decorators.open_access
@@ -303,8 +304,7 @@ def get(self):
 
 
 class CollectionSummariesHandler(base.BaseHandler):
-    """Returns collection summaries corresponding to collection ids.
-    """
+    """Returns collection summaries corresponding to collection ids."""
 
     GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
 

core/controllers/library_test.py

@@ -91,7 +91,7 @@ def test_library_handler_demo_exploration(self):
             self.count_jobs_in_taskqueue(
                 taskqueue_services.QUEUE_NAME_ONE_OFF_JOBS), 0)
 
-        # change title and category
+        # change title and category.
         exp_services.update_exploration(
             self.editor_id, '0', [{
                 'cmd': 'edit_exploration_property',
@@ -132,7 +132,7 @@ def test_library_handler_for_created_explorations(self):
             'search_cursor': None,
         }, response_dict)
 
-        # Create exploration A
+        # Create exploration A.
         exploration = self.save_new_valid_exploration(
             'A', self.admin_id, title='Title A', category='Category A',
             objective='Objective A')
@@ -143,15 +143,15 @@ def test_library_handler_for_created_explorations(self):
         response_dict = self.get_json(feconf.LIBRARY_SEARCH_DATA_URL)
         self.assertEqual(response_dict['activity_list'], [])
 
-        # Create exploration B
+        # Create exploration B.
         exploration = self.save_new_valid_exploration(
             'B', self.admin_id, title='Title B', category='Category B',
             objective='Objective B')
         exp_services._save_exploration(  # pylint: disable=protected-access
             self.admin_id, exploration, 'Exploration B', [])
         rights_manager.publish_exploration(self.admin, 'B')
 
-        # Publish exploration A
+        # Publish exploration A.
         rights_manager.publish_exploration(self.admin, 'A')
 
         exp_services.index_explorations_given_ids(['A', 'B'])
@@ -176,7 +176,7 @@ def test_library_handler_for_created_explorations(self):
             'status': rights_manager.ACTIVITY_STATUS_PUBLIC,
         }, response_dict['activity_list'][0])
 
-        # Delete exploration A
+        # Delete exploration A.
         exp_services.delete_exploration(self.admin_id, 'A')
 
         # Load the search results with an empty query.