From 9ca7e086e3d9edbf60ceaa35aa777a0ad55c33d9 Mon Sep 17 00:00:00 2001 From: Apurv-Bajaj Date: Mon, 14 May 2018 20:39:28 +0530 Subject: [PATCH 1/2] Enable pycodestyle rules and fix errors --- core/controllers/classifier_test.py | 10 ++++---- core/controllers/collection_editor_test.py | 6 ++--- core/controllers/creator_dashboard_test.py | 4 ++-- core/controllers/editor_test.py | 2 +- core/controllers/pages.py | 1 + core/domain/acl_decorators.py | 3 +++ core/domain/classifier_domain.py | 4 ++-- core/domain/classifier_services_test.py | 7 +++--- core/domain/collection_domain.py | 4 ++-- core/domain/collection_services_test.py | 2 +- core/domain/email_manager_test.py | 2 +- core/domain/exp_domain.py | 3 +++ core/domain/exp_domain_test.py | 1 + core/domain/exp_services.py | 6 ++--- core/domain/exp_services_test.py | 6 ++++- core/domain/fs_domain.py | 1 + core/domain/question_domain.py | 1 + core/domain/stats_domain.py | 5 ++++ core/domain/stats_jobs_continuous.py | 2 +- core/domain/stats_jobs_one_off.py | 2 +- core/domain/stats_services_test.py | 3 ++- core/domain/summary_services_test.py | 2 +- core/domain/user_jobs_continuous.py | 6 ++--- core/domain/user_jobs_continuous_test.py | 10 ++++---- core/domain/user_query_jobs_one_off_test.py | 2 +- core/domain/user_services.py | 1 + core/jobs.py | 3 +++ core/platform/email/mailgun_email_services.py | 2 +- core/platform/search/gae_search_services.py | 1 + core/storage/user/gae_models_test.py | 1 + .../performance_framework/perf_domain.py | 19 ++++++++------- core/tests/test_utils_test.py | 2 +- extensions/answer_summarizers/models_test.py | 24 +++++++++---------- scripts/backend_tests.py | 10 ++++---- scripts/common.py | 1 + scripts/cut_release_branch.py | 6 ++--- scripts/install_third_party.py | 2 +- scripts/pre_commit_linter.py | 10 ++++---- scripts/pre_push_hook.py | 12 +++++----- tox.ini | 2 +- 40 files changed, 110 insertions(+), 81 deletions(-) diff --git a/core/controllers/classifier_test.py b/core/controllers/classifier_test.py index e240a39128d9..fbf4683b19bd 100644 --- a/core/controllers/classifier_test.py +++ b/core/controllers/classifier_test.py @@ -86,8 +86,8 @@ def setUp(self): classifier_training_job_model.put() self.job_result_dict = { - 'job_id' : self.job_id, - 'classifier_data' : self.classifier_data, + 'job_id': self.job_id, + 'classifier_data': self.classifier_data, } self.payload = {} @@ -165,9 +165,9 @@ def setUp(self): ) self.expected_response = { - u'job_id' : unicode(self.job_id, 'utf-8'), - u'training_data' : self.training_data, - u'algorithm_id' : unicode(self.algorithm_id, 'utf-8') + u'job_id': unicode(self.job_id, 'utf-8'), + u'training_data': self.training_data, + u'algorithm_id': unicode(self.algorithm_id, 'utf-8') } self.payload = {} diff --git a/core/controllers/collection_editor_test.py b/core/controllers/collection_editor_test.py index 0441a500d11e..fb56b8c38ff4 100644 --- a/core/controllers/collection_editor_test.py +++ b/core/controllers/collection_editor_test.py @@ -43,9 +43,9 @@ def setUp(self): self.admin = user_services.UserActionsInfo(self.admin_id) self.json_dict = { - 'version' : 1, - 'commit_message' : 'changed title', - 'change_list' : [{ + 'version': 1, + 'commit_message': 'changed title', + 'change_list': [{ 'cmd': 'edit_collection_property', 'property_name': 'title', 'new_value': 'A new title' diff --git a/core/controllers/creator_dashboard_test.py b/core/controllers/creator_dashboard_test.py index e11434b5b8d8..99f5225395b5 100644 --- a/core/controllers/creator_dashboard_test.py +++ b/core/controllers/creator_dashboard_test.py @@ -298,7 +298,7 @@ def test_multiple_plays_and_ratings_for_multiple_explorations(self): self.assertEquals( user_model.impact_score, self.USER_IMPACT_SCORE_DEFAULT) self.assertEquals(user_model.num_ratings, 3) - self.assertEquals(user_model.average_ratings, 10/3.0) + self.assertEquals(user_model.average_ratings, 10 / 3.0) self.logout() def test_stats_for_single_exploration_with_multiple_owners(self): @@ -386,7 +386,7 @@ def test_stats_for_multiple_explorations_with_multiple_owners(self): expected_results = { 'total_plays': 5, 'num_ratings': 4, - 'average_ratings': 18/4.0 + 'average_ratings': 18 / 4.0 } user_model_2 = user_models.UserStatsModel.get(self.owner_id_2) diff --git a/core/controllers/editor_test.py b/core/controllers/editor_test.py index 9b5ee489b744..29ffd7d9d95d 100644 --- a/core/controllers/editor_test.py +++ b/core/controllers/editor_test.py @@ -1659,7 +1659,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, diff --git a/core/controllers/pages.py b/core/controllers/pages.py index 799a1de7f4c4..b6e70f867cfc 100644 --- a/core/controllers/pages.py +++ b/core/controllers/pages.py @@ -25,6 +25,7 @@ # TODO(bhenning): Convert this over to using action-based ACLs. def require_maintenance_mode(handler): """Decorator that checks whether maintenance mode is enabled in feconf.""" + def test_maintenance_mode(self, **kwargs): """Checks whether the site is in maintenance mode.""" if not feconf.ENABLE_MAINTENANCE_MODE: diff --git a/core/domain/acl_decorators.py b/core/domain/acl_decorators.py index 155919529051..a60892fd03fb 100644 --- a/core/domain/acl_decorators.py +++ b/core/domain/acl_decorators.py @@ -236,6 +236,7 @@ def test_can_manage_profile(self, **kwargs): def can_access_admin_page(handler): """Decorator that checks if the current user is a super admin.""" + def test_super_admin(self, **kwargs): """Checks if the user is logged in and is a super admin.""" if not self.user_id: @@ -467,6 +468,7 @@ def can_suggest_changes_to_exploration(handler): """Decorator to check whether a user can make suggestions to an exploration. """ + def test_can_suggest(self, exploration_id, **kwargs): """Checks if the user can make suggestions to an exploration. @@ -626,6 +628,7 @@ def require_user_id_else_redirect_to_homepage(handler): session. If not, the user is redirected to the main page. Note that the user may not yet have registered. """ + def test_login(self, **kwargs): """Checks if the user for the current session is logged in. If not, redirects the user to the home page. diff --git a/core/domain/classifier_domain.py b/core/domain/classifier_domain.py index 6f8232f56569..64abfcdb51f7 100644 --- a/core/domain/classifier_domain.py +++ b/core/domain/classifier_domain.py @@ -359,7 +359,7 @@ def validate(self): if not isinstance(self.training_data, list): raise utils.ValidationError( - 'Expected training_data to be a list, received %s' %( + 'Expected training_data to be a list, received %s' % ( self.training_data)) for grouped_answers in self.training_data: @@ -382,7 +382,7 @@ def validate(self): # Classifier data can be either None (before its stored) or a dict. if not isinstance(self.classifier_data, dict) and self.classifier_data: raise utils.ValidationError( - 'Expected classifier_data to be a dict|None, received %s' %( + 'Expected classifier_data to be a dict|None, received %s' % ( self.classifier_data)) if not isinstance(self.data_schema_version, int): diff --git a/core/domain/classifier_services_test.py b/core/domain/classifier_services_test.py index 050e4fbe0711..4efc367f0e09 100644 --- a/core/domain/classifier_services_test.py +++ b/core/domain/classifier_services_test.py @@ -38,6 +38,7 @@ class ClassifierServicesTests(test_utils.GenericTestBase): test hard rules, ReaderClassifyTests is only checking that the string classifier is actually called. """ + def setUp(self): super(ClassifierServicesTests, self).setUp() self._init_classify_inputs('16') @@ -189,11 +190,11 @@ def test_handle_non_retrainable_states(self): # Create job and mapping for previous version. job_id = classifier_models.ClassifierTrainingJobModel.create( feconf.INTERACTION_CLASSIFIER_MAPPING['TextInput']['algorithm_id'], - 'TextInput', self.exp_id, exploration.version-1, + 'TextInput', self.exp_id, exploration.version - 1, next_scheduled_check_time, [], 'Old home', feconf.TRAINING_JOB_STATUS_COMPLETE, None, 1) classifier_models.TrainingJobExplorationMappingModel.create( - self.exp_id, exploration.version-1, 'Old home', job_id) + self.exp_id, exploration.version - 1, 'Old home', job_id) classifier_services.handle_non_retrainable_states( exploration, state_names, exp_versions_diff) @@ -264,7 +265,7 @@ def test_deletion_of_classifier_training_jobs(self): classifier_services.delete_classifier_training_job(job_id) with self.assertRaisesRegexp(Exception, ( 'Entity for class ClassifierTrainingJobModel ' - 'with id %s not found' %( + 'with id %s not found' % ( job_id))): classifier_services.get_classifier_training_job_by_id(job_id) diff --git a/core/domain/collection_domain.py b/core/domain/collection_domain.py index 3f2b5a31a183..33eec737bbdd 100644 --- a/core/domain/collection_domain.py +++ b/core/domain/collection_domain.py @@ -775,9 +775,9 @@ def get_next_exploration_id_in_sequence(self, current_exploration_id): """ exploration_just_unlocked = None - for index in range(0, len(self.nodes)-1): + for index in range(0, len(self.nodes) - 1): if self.nodes[index].exploration_id == current_exploration_id: - exploration_just_unlocked = self.nodes[index+1].exploration_id + exploration_just_unlocked = self.nodes[index + 1].exploration_id break return exploration_just_unlocked diff --git a/core/domain/collection_services_test.py b/core/domain/collection_services_test.py index 33c32d9810b0..e7683448c7ce 100644 --- a/core/domain/collection_services_test.py +++ b/core/domain/collection_services_test.py @@ -1524,7 +1524,7 @@ def test_contributor_summary(self): 'property_name': 'title', 'new_value': 'Collection Bob title' }] - # Have Bob update that collection. Version 2. + # Have Bob update that collection. Version 2. collection_services.update_collection( bob_id, self.COLLECTION_ID, changelist_cmds, 'Changed title.') self._check_contributors_summary( diff --git a/core/domain/email_manager_test.py b/core/domain/email_manager_test.py index a9255d6788e6..c9b7b68d46c4 100644 --- a/core/domain/email_manager_test.py +++ b/core/domain/email_manager_test.py @@ -1144,7 +1144,7 @@ def test_correct_email_body_is_sent(self): 'You can change your email preferences via the Preferences page.') feedback_messages = { - self.exploration.id : { + self.exploration.id: { 'title': self.exploration.title, 'messages': ['Message 1.1', 'Message 1.2', 'Message 1.3']} } diff --git a/core/domain/exp_domain.py b/core/domain/exp_domain.py index e175a55d4835..0cfe4a1694d6 100644 --- a/core/domain/exp_domain.py +++ b/core/domain/exp_domain.py @@ -707,6 +707,7 @@ class Outcome(object): consists of a destination state, feedback to show the user, and any parameter changes. """ + def to_dict(self): """Returns a dict representing this Outcome domain object. @@ -819,6 +820,7 @@ class AnswerGroup(object): that involve soft matching of answers to a set of training data and/or example answers dictated by the creator. """ + def to_dict(self): """Returns a dict representing this AnswerGroup domain object. @@ -950,6 +952,7 @@ class Solution(object): to progress to the next card and explanation is an HTML string containing an explanation for the solution. """ + def __init__( self, interaction_id, answer_is_exclusive, correct_answer, explanation): diff --git a/core/domain/exp_domain_test.py b/core/domain/exp_domain_test.py index fb0bf733e4af..521f0f6ce5d3 100644 --- a/core/domain/exp_domain_test.py +++ b/core/domain/exp_domain_test.py @@ -957,6 +957,7 @@ class SchemaMigrationMethodsUnitTests(test_utils.GenericTestBase): """Tests the presence of appropriate schema migration methods in the Exploration domain object class. """ + def test_correct_states_schema_conversion_methods_exist(self): """Test that the right states schema conversion methods exist.""" current_states_schema_version = ( diff --git a/core/domain/exp_services.py b/core/domain/exp_services.py index c3cd7b8d0b09..6401c6347495 100644 --- a/core/domain/exp_services.py +++ b/core/domain/exp_services.py @@ -1669,9 +1669,9 @@ def get_scaled_average_rating(ratings): x = (average_rating - 1) / 4 # The following calculates the lower bound Wilson Score as documented # http://www.goproblems.com/test/wilson/wilson.php?v1=0&v2=0&v3=0&v4=&v5=1 - a = x + (z**2)/(2*n) - b = z * math.sqrt((x*(1-x))/n + (z**2)/(4*n**2)) - wilson_score_lower_bound = (a - b)/(1 + z**2/n) + a = x + (z**2) / (2 * n) + b = z * math.sqrt((x * (1 - x)) / n + (z**2) / (4 * n**2)) + wilson_score_lower_bound = (a - b) / (1 + z**2 / n) return 1 + 4 * wilson_score_lower_bound diff --git a/core/domain/exp_services_test.py b/core/domain/exp_services_test.py index fb5bf52caf44..d5b61f19b170 100644 --- a/core/domain/exp_services_test.py +++ b/core/domain/exp_services_test.py @@ -80,6 +80,7 @@ def setUp(self): class ExplorationQueriesUnitTests(ExplorationServicesUnitTests): """Tests query methods.""" + def test_get_exploration_titles_and_categories(self): self.assertEqual( exp_services.get_exploration_titles_and_categories([]), {}) @@ -310,6 +311,7 @@ def test_exploration_summaries_pagination_in_filled_search_results(self): class ExplorationCreateAndDeleteUnitTests(ExplorationServicesUnitTests): """Test creation and deletion methods.""" + def test_retrieval_of_explorations(self): """Test the get_exploration_by_id() method.""" with self.assertRaisesRegexp(Exception, 'Entity .* not found'): @@ -1257,6 +1259,7 @@ def _get_change_list(state_name, property_name, new_value): class UpdateStateTests(ExplorationServicesUnitTests): """Test updating a single state.""" + def setUp(self): super(UpdateStateTests, self).setUp() exploration = self.save_new_valid_exploration( @@ -1602,6 +1605,7 @@ def test_update_content_missing_key(self): class CommitMessageHandlingTests(ExplorationServicesUnitTests): """Test the handling of commit messages.""" + def setUp(self): super(CommitMessageHandlingTests, self).setUp() exploration = self.save_new_valid_exploration( @@ -2319,7 +2323,7 @@ def test_contributors_summary(self): self.save_new_valid_exploration(self.EXP_ID_1, albert_id) self._check_contributors_summary(self.EXP_ID_1, {albert_id: 1}) - # Have Bob update that exploration. Version 2. + # Have Bob update that exploration. Version 2. exp_services.update_exploration( bob_id, self.EXP_ID_1, [{ 'cmd': exp_domain.CMD_EDIT_EXPLORATION_PROPERTY, diff --git a/core/domain/fs_domain.py b/core/domain/fs_domain.py index b75857f2432d..b233114e4e0a 100644 --- a/core/domain/fs_domain.py +++ b/core/domain/fs_domain.py @@ -39,6 +39,7 @@ class FileMetadata(object): Attributes: size: int. The size of the file, in bytes. """ + def __init__(self, metadata): """Constructs a FileMetadata object. diff --git a/core/domain/question_domain.py b/core/domain/question_domain.py index 8f74a1db73f2..626f15f32074 100644 --- a/core/domain/question_domain.py +++ b/core/domain/question_domain.py @@ -232,6 +232,7 @@ class QuestionSummary(object): question_id: str. The ID of the question. question_title: str. The title of the question. """ + def __init__(self, question_id, question_title): """Constructs a Question Summary domain object. diff --git a/core/domain/stats_domain.py b/core/domain/stats_domain.py index 64aa1da0f48a..d46122c414c0 100644 --- a/core/domain/stats_domain.py +++ b/core/domain/stats_domain.py @@ -640,6 +640,7 @@ class AnswerOccurrence(object): """Domain object that represents a specific answer that occurred some number of times. """ + def __init__(self, answer, frequency): """Initialize domain object for answer occurrences.""" self.answer = answer @@ -685,12 +686,14 @@ class AnswerCalculationOutput(object): """Domain object superclass that represents the output of an answer calculation. """ + def __init__(self, calculation_output_type): self.calculation_output_type = calculation_output_type class AnswerFrequencyList(AnswerCalculationOutput): """Domain object that represents an output list of AnswerOccurrences.""" + def __init__(self, answer_occurrences=None): """Initialize domain object for answer frequency list for a given list of AnswerOccurrence objects (default is empty list). @@ -741,6 +744,7 @@ class CategorizedAnswerFrequencyLists(AnswerCalculationOutput): """AnswerFrequencyLists that are categorized based on arbitrary categories. """ + def __init__(self, categorized_answer_freq_lists=None): """Initialize domain object for categorized answer frequency lists for a given dict (default is empty). @@ -799,6 +803,7 @@ class StateAnswersCalcOutput(object): """Domain object that represents output of calculations operating on state answers. """ + def __init__( self, exploration_id, exploration_version, state_name, interaction_id, calculation_id, calculation_output): diff --git a/core/domain/stats_jobs_continuous.py b/core/domain/stats_jobs_continuous.py index 271ada7ecbab..77c23b42c1a9 100644 --- a/core/domain/stats_jobs_continuous.py +++ b/core/domain/stats_jobs_continuous.py @@ -208,7 +208,7 @@ def reduce(key, stringified_values): for ignored_version in ignored_versions: del versioned_interaction_ids[ignored_version] del versioned_item_ids[ignored_version] - versions = versions[:earliest_acceptable_version_index+1] + versions = versions[:earliest_acceptable_version_index + 1] # Retrieve all StateAnswerModel entities associated with the remaining # item IDs which correspond to a single interaction ID shared among all diff --git a/core/domain/stats_jobs_one_off.py b/core/domain/stats_jobs_one_off.py index c5d9b5060bde..e3b98441ad89 100644 --- a/core/domain/stats_jobs_one_off.py +++ b/core/domain/stats_jobs_one_off.py @@ -131,7 +131,7 @@ def reduce(exp_id, values): event_dict_idx = 0 event_dict = sorted_events_dicts[event_dict_idx] for version in versions: - datastore_stats_for_version = old_stats[version-1] + datastore_stats_for_version = old_stats[version - 1] if version == 1: # Reset the possibly corrupted stats. datastore_stats_for_version.num_starts_v2 = 0 diff --git a/core/domain/stats_services_test.py b/core/domain/stats_services_test.py index b0d55b41409a..a9d3d0ff16b0 100644 --- a/core/domain/stats_services_test.py +++ b/core/domain/stats_services_test.py @@ -38,6 +38,7 @@ class StatisticsServicesTest(test_utils.GenericTestBase): """Test the helper functions and methods defined in the stats_services module. """ + def setUp(self): super(StatisticsServicesTest, self).setUp() self.exp_id = 'exp_id1' @@ -407,7 +408,7 @@ def test_create_stats_model(self): exploration_stats.exp_version += 1 model_id = stats_services.create_stats_model(exploration_stats) exploration_stats = stats_services.get_exploration_stats_by_id( - self.exp_id, self.exp_version+1) + self.exp_id, self.exp_version + 1) self.assertEqual(exploration_stats.exp_id, 'exp_id1') self.assertEqual(exploration_stats.exp_version, 2) self.assertEqual(exploration_stats.num_starts_v1, 0) diff --git a/core/domain/summary_services_test.py b/core/domain/summary_services_test.py index 9d2bd55979af..a298d9fb0d56 100644 --- a/core/domain/summary_services_test.py +++ b/core/domain/summary_services_test.py @@ -274,7 +274,7 @@ def test_get_library_groups(self): 'ratings': feconf.get_empty_ratings(), 'status': u'public', 'tags': [], - 'title': u'The Lazy Magician', + 'title': u'The Lazy Magician', 'thumbnail_bg_color': '#d0982a', 'thumbnail_icon_url': '/subjects/Algorithms.svg', } diff --git a/core/domain/user_jobs_continuous.py b/core/domain/user_jobs_continuous.py index 67cacc7583e9..1391e8b1ce26 100644 --- a/core/domain/user_jobs_continuous.py +++ b/core/domain/user_jobs_continuous.py @@ -408,7 +408,7 @@ def _refresh_average_ratings(user_id, rating, old_rating): if old_rating is not None: sum_of_ratings -= old_rating num_ratings -= 1 - model.average_ratings = sum_of_ratings/(num_ratings * 1.0) + model.average_ratings = sum_of_ratings / (num_ratings * 1.0) else: model.average_ratings = rating model.num_ratings = num_ratings @@ -542,7 +542,7 @@ def map(item): if item.deleted: return - exponent = 2.0/3 + exponent = 2.0 / 3 # This is set to False only when the exploration impact score is not # valid to be calculated. @@ -655,7 +655,7 @@ def reduce(key, stringified_values): all explorations owned by the user. """ values = [ast.literal_eval(v) for v in stringified_values] - exponent = 2.0/3 + exponent = 2.0 / 3 # Find the final score and round to a whole number. user_impact_score = int(round(sum( diff --git a/core/domain/user_jobs_continuous_test.py b/core/domain/user_jobs_continuous_test.py index 5ea901cd826a..d7c73c9ed3bb 100644 --- a/core/domain/user_jobs_continuous_test.py +++ b/core/domain/user_jobs_continuous_test.py @@ -595,7 +595,7 @@ class UserStatsAggregatorTest(test_utils.GenericTestBase): USER_B_USERNAME = 'b' MIN_NUM_COMPLETIONS = 2 - EXPONENT = 2.0/3 + EXPONENT = 2.0 / 3 def setUp(self): super(UserStatsAggregatorTest, self).setUp() @@ -886,7 +886,7 @@ def test_realtime_layer_batch_job_single_exploration_multiple_owners(self): expected_results = { 'total_plays': 2, 'num_ratings': 6, - 'average_ratings': 22/6.0 + 'average_ratings': 22 / 6.0 } user_stats_1 = ( @@ -925,7 +925,7 @@ def test_realtime_layer_batch_job_multiple_explorations_one_owner(self): self.user_a_id)) self.assertEquals(user_stats['total_plays'], 0) self.assertEquals(user_stats['num_ratings'], 5) - self.assertEquals(user_stats['average_ratings'], 18/5.0) + self.assertEquals(user_stats['average_ratings'], 18 / 5.0) def test_realtime_layer_batch_job_user_rate_same_exp_multiple_times(self): self._create_exploration( @@ -971,7 +971,7 @@ def test_both_realtime_layer_and_batch_data(self): # _mock_get_statistics() method above. self.assertEqual(user_stats_model.total_plays, 14) self.assertEqual(user_stats_model.num_ratings, 6) - self.assertEqual(user_stats_model.average_ratings, 20/6.0) + self.assertEqual(user_stats_model.average_ratings, 20 / 6.0) # Stop the batch job. Fire up a few events and check data from realtime # job. @@ -988,4 +988,4 @@ def test_both_realtime_layer_and_batch_data(self): # two. self.assertEquals(user_stats['total_plays'], 16) self.assertEquals(user_stats['num_ratings'], 10) - self.assertEquals(user_stats['average_ratings'], 32/10.0) + self.assertEquals(user_stats['average_ratings'], 32 / 10.0) diff --git a/core/domain/user_query_jobs_one_off_test.py b/core/domain/user_query_jobs_one_off_test.py index 5f05ea1d41f2..07deb2e2d11d 100644 --- a/core/domain/user_query_jobs_one_off_test.py +++ b/core/domain/user_query_jobs_one_off_test.py @@ -292,7 +292,7 @@ def test_that_correct_email_is_sent_upon_completion(self): '\n' 'You can change your email preferences via the ' 'Preferences page.' - ) % query_id + ) % query_id messages = self.mail_stub.get_sent_messages( to=self.USER_SUBMITTER_EMAIL) diff --git a/core/domain/user_services.py b/core/domain/user_services.py index 5f73b612de6e..4161458aca8f 100644 --- a/core/domain/user_services.py +++ b/core/domain/user_services.py @@ -78,6 +78,7 @@ class UserSettings(object): preferred_site_language_code: str or None. System language preference. preferred_audio_language_code: str or None. Audio language preference. """ + def __init__( self, user_id, email, role, username=None, last_agreed_to_terms=None, last_started_state_editor_tutorial=None, diff --git a/core/jobs.py b/core/jobs.py index 76b6a6f446c8..898a0fd4c815 100644 --- a/core/jobs.py +++ b/core/jobs.py @@ -587,6 +587,7 @@ class MapReduceJobPipeline(base_handler.PipelineBase): a run method which is called when this job is started by using start() method on the object created from this class. """ + def run(self, job_id, job_class_str, kwargs): """Returns a coroutine which runs the job pipeline and stores results. @@ -619,6 +620,7 @@ def finalized(self): class StoreMapReduceResults(base_handler.PipelineBase): """MapreducePipeline class to store output results.""" + def run(self, job_id, job_class_str, output): """Extracts the results of a MR job and registers its completion. @@ -654,6 +656,7 @@ class GoogleCloudStorageConsistentJsonOutputWriter( preferred as it's consistent. For more details please look here https://github.com/GoogleCloudPlatform/appengine-mapreduce/wiki/3.4-Readers-and-Writers#googlecloudstorageoutputwriter """ + def write(self, data): """Writes that data serialized in JSON format. diff --git a/core/platform/email/mailgun_email_services.py b/core/platform/email/mailgun_email_services.py index 95fdb0af59d3..6740c0a4f177 100644 --- a/core/platform/email/mailgun_email_services.py +++ b/core/platform/email/mailgun_email_services.py @@ -122,7 +122,7 @@ def send_bulk_mail( # For more detail check following link: # https://documentation.mailgun.com/user_manual.html#batch-sending recipient_email_sets = [ - recipient_emails[i:i+1000] + recipient_emails[i:i + 1000] for i in xrange(0, len(recipient_emails), 1000)] for email_set in recipient_email_sets: diff --git a/core/platform/search/gae_search_services.py b/core/platform/search/gae_search_services.py index e3c1a4f4eb5d..527c0475c4d3 100644 --- a/core/platform/search/gae_search_services.py +++ b/core/platform/search/gae_search_services.py @@ -33,6 +33,7 @@ class SearchFailureError(Exception): Other platform implementations should have a similar way of revealing platform specific errors. """ + def __init__(self, original_exception=None): super(SearchFailureError, self).__init__( '%s: %s' % (type(original_exception), original_exception.message)) diff --git a/core/storage/user/gae_models_test.py b/core/storage/user/gae_models_test.py index 590d92046adb..d15201842a7b 100644 --- a/core/storage/user/gae_models_test.py +++ b/core/storage/user/gae_models_test.py @@ -135,6 +135,7 @@ def test_get_failure(self): class UserQueryModelTests(test_utils.GenericTestBase): """Tests for UserQueryModel.""" + def test_instance_stores_correct_data(self): submitter_id = 'submitter' query_id = 'qid' diff --git a/core/tests/performance_framework/perf_domain.py b/core/tests/performance_framework/perf_domain.py index c7a0b3fa4db6..b361ce306882 100644 --- a/core/tests/performance_framework/perf_domain.py +++ b/core/tests/performance_framework/perf_domain.py @@ -236,8 +236,8 @@ def print_details(self): """Helper function to print details for all the events.""" if self.page_session_stats: print 'Total number of requests: %d' % self.get_request_count() - print ('Total page size in bytes: %d' - % self.get_total_page_size_bytes()) + print('Total page size in bytes: %d' + % self.get_total_page_size_bytes()) else: print 'Page session stats are not available.' @@ -248,13 +248,13 @@ def print_details(self): print 'Ready start time: %d' % self.get_ready_start_time_millisecs() print 'Redirect time: %d' % self.get_redirect_time_millisecs() print 'Appcache time: %d' % self.get_appcache_time_millisecs() - print ('Unload event time: %d' - % self.get_unload_event_time_millisecs()) + print('Unload event time: %d' + % self.get_unload_event_time_millisecs()) print 'DNS query time: %d' % self.get_lookup_domain_time_millisecs() - print ('TCP connection time: %d' - % self.get_connect_time_millisecs()) - print ('Init domtree time: %d' - % self.get_init_dom_tree_time_millisecs()) + print('TCP connection time: %d' + % self.get_connect_time_millisecs()) + print('Init domtree time: %d' + % self.get_init_dom_tree_time_millisecs()) print 'Load event time: %d' % self.get_load_event_time_millisecs() else: print 'Page session timings are not available.' @@ -266,7 +266,8 @@ class MultiplePageSessionMetrics(object): different page load sessions. This may happen due to various factors like background processes. """ - def __init__(self, page_session_metrics): + + def __init__(self, page_session_metrics): self.page_metrics = page_session_metrics self._validate() diff --git a/core/tests/test_utils_test.py b/core/tests/test_utils_test.py index fec246c180f1..5b79be52a65c 100644 --- a/core/tests/test_utils_test.py +++ b/core/tests/test_utils_test.py @@ -113,7 +113,7 @@ def side_effect(num): data['value'] = num return num - l = lambda x: side_effect(x)*2 + l = lambda x: side_effect(x) * 2 wrapped = test_utils.FunctionWrapper(l) self.assertEqual(wrapped('foobar'), 'foobarfoobar') diff --git a/extensions/answer_summarizers/models_test.py b/extensions/answer_summarizers/models_test.py index a2916e34173f..7e3baa4edac2 100644 --- a/extensions/answer_summarizers/models_test.py +++ b/extensions/answer_summarizers/models_test.py @@ -88,8 +88,8 @@ def test_top_answers_without_ties(self): # Create 12 answers with different frequencies. answers = ( ['A'] * 12 + ['B'] * 11 + ['C'] * 10 + ['D'] * 9 + - ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + - ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) + ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + + ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) answer_dicts_list = [self._create_answer_dict(a) for a in answers] state_answers_dict = self._create_state_answers_dict(answer_dicts_list) @@ -146,8 +146,8 @@ def test_top5_without_ties(self): # Create 12 answers with different frequencies. answers = ( ['A'] * 12 + ['B'] * 11 + ['C'] * 10 + ['D'] * 9 + - ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + - ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) + ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + + ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) answer_dicts_list = [self._create_answer_dict(a) for a in answers] state_answers_dict = self._create_state_answers_dict(answer_dicts_list) @@ -190,8 +190,8 @@ def test_top10_answers_without_ties(self): # Create 12 answers with different frequencies. answers = ( ['A'] * 12 + ['B'] * 11 + ['C'] * 10 + ['D'] * 9 + - ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + - ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) + ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + + ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) answer_dicts_list = [self._create_answer_dict(a) for a in answers] state_answers_dict = self._create_state_answers_dict(answer_dicts_list) @@ -260,14 +260,14 @@ def test_shared_answers(self): def test_many_shared_answers(self): answers = ( ['A'] * 12 + ['B'] * 11 + ['C'] * 10 + ['D'] * 9 + - ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + - ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) + ['E'] * 8 + ['F'] * 7 + ['G'] * 6 + ['H'] * 5 + + ['I'] * 4 + ['J'] * 3 + ['K'] * 2 + ['L']) split_len = len(answers) // 4 answer_dicts_list = [ - self._create_answer_dict(answers[ :split_len*1]), - self._create_answer_dict(answers[split_len*1:split_len*2]), - self._create_answer_dict(answers[split_len*2:split_len*3]), - self._create_answer_dict(answers[split_len*3: ]), + self._create_answer_dict(answers[:split_len * 1]), + self._create_answer_dict(answers[split_len * 1:split_len * 2]), + self._create_answer_dict(answers[split_len * 2:split_len * 3]), + self._create_answer_dict(answers[split_len * 3:]), ] state_answers_dict = self._create_state_answers_dict(answer_dicts_list) diff --git a/scripts/backend_tests.py b/scripts/backend_tests.py index 70c725eb1c1d..8d7bb40a90a1 100644 --- a/scripts/backend_tests.py +++ b/scripts/backend_tests.py @@ -93,7 +93,7 @@ def run_shell_cmd(exe, stdout=subprocess.PIPE, stderr=subprocess.PIPE): log('') for line in last_stdout: if line.startswith(LOG_LINE_PREFIX): - log('INFO: %s' % line[len(LOG_LINE_PREFIX): ]) + log('INFO: %s' % line[len(LOG_LINE_PREFIX):]) log('') result = '%s%s' % (last_stdout_str, last_stderr_str) @@ -297,7 +297,7 @@ def main(): test_count = 0 elif task.exception: exc_str = str(task.exception).decode('utf-8') - print exc_str[exc_str.find('=') : exc_str.rfind('-')] + print exc_str[exc_str.find('='): exc_str.rfind('-')] tests_failed_regex_match = re.search( r'Test suite failed: ([0-9]+) tests run, ([0-9]+) errors, ' @@ -328,10 +328,10 @@ def main(): r'Ran ([0-9]+) tests? in ([0-9\.]+)s', task.output) test_count = int(tests_run_regex_match.group(1)) test_time = float(tests_run_regex_match.group(2)) - print ('SUCCESS %s: %d tests (%.1f secs)' % - (spec.test_target, test_count, test_time)) + print('SUCCESS %s: %d tests (%.1f secs)' % + (spec.test_target, test_count, test_time)) except Exception: - print ( + print( 'An unexpected error occurred. ' 'Task output:\n%s' % task.output) diff --git a/scripts/common.py b/scripts/common.py index 1ecd7d7ebbf9..ca1f8c69a5d9 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -134,6 +134,7 @@ def ensure_release_scripts_folder_exists_and_is_up_to_date(): class CD(object): """Context manager for changing the current working directory.""" + def __init__(self, new_path): self.new_path = new_path self.saved_path = None diff --git a/scripts/cut_release_branch.py b/scripts/cut_release_branch.py index dcb2654fef0f..12f2a6ed3b60 100644 --- a/scripts/cut_release_branch.py +++ b/scripts/cut_release_branch.py @@ -125,13 +125,13 @@ def _execute_branch_cut(): common.open_new_tab_in_browser_if_possible( 'https://github.com/oppia/oppia#oppia---') while True: - print ( + print( 'Please confirm: are Travis checks passing on develop? (y/n) ') answer = raw_input().lower() if answer in ['y', 'ye', 'yes']: break elif answer: - print ( + print( 'Tests should pass on develop before this script is run. ' 'Exiting.') sys.exit() @@ -163,7 +163,7 @@ def _execute_branch_cut(): subprocess.call(['git', 'push', remote_alias, NEW_BRANCH_NAME]) print '' - print ( + print( 'New release branch successfully cut. You are now on branch %s' % NEW_BRANCH_NAME) print 'Done!' diff --git a/scripts/install_third_party.py b/scripts/install_third_party.py index 52bd942eb841..ef72b3d12548 100644 --- a/scripts/install_third_party.py +++ b/scripts/install_third_party.py @@ -227,7 +227,7 @@ def test_manifest_syntax(dependency_type, dependency_dict): print '------------------------------------------' print 'There is syntax error in this dependency' print dependency_dict - print ( + print( 'Only one of these keys pair must be used: "%s".' % str(optional_keys)) print 'Exiting' diff --git a/scripts/pre_commit_linter.py b/scripts/pre_commit_linter.py index 00184082152f..d74448904b80 100644 --- a/scripts/pre_commit_linter.py +++ b/scripts/pre_commit_linter.py @@ -378,7 +378,7 @@ def _lint_py_files(config_pylint, config_pycodestyle, files_to_lint, result): current_batch_end_index = min( current_batch_start_index + _BATCH_SIZE, len(files_to_lint)) current_files_to_lint = files_to_lint[ - current_batch_start_index : current_batch_end_index] + current_batch_start_index: current_batch_end_index] print 'Linting Python files %s to %s...' % ( current_batch_start_index + 1, current_batch_end_index) @@ -486,8 +486,8 @@ def _get_all_files(): else: invalid_filepaths.append(f) if invalid_filepaths: - print ('The following file(s) do not exist: %s\n' - 'Exiting.' % invalid_filepaths) + print('The following file(s) do not exist: %s\n' + 'Exiting.' % invalid_filepaths) sys.exit(1) all_files = valid_filepaths else: @@ -592,7 +592,7 @@ def _check_newline_character(all_files): f.seek(-2, 2) if not (f.read(1) != '\n' and f.read(1) == '\n'): failed = True - print ( + print( '%s --> Please ensure that this file ends' 'with exactly one newline char.' % filename) total_error_count += 1 @@ -922,7 +922,7 @@ def _check_html_directive_name(all_files): if not directive_filename.endswith('_directive.html'): failed = True total_error_count += 1 - print ( + print( '%s --> Please ensure that this file ends' 'with _directive.html.' % directive_filename) if failed: diff --git a/scripts/pre_push_hook.py b/scripts/pre_push_hook.py index 5b5321c0aa53..287ef6de4f91 100755 --- a/scripts/pre_push_hook.py +++ b/scripts/pre_push_hook.py @@ -71,10 +71,10 @@ def __enter__(self): subprocess.check_output( ['git', 'checkout', self.new_branch, '--']) except subprocess.CalledProcessError: - print ('\nCould not change branch to %s. This is most probably ' - 'because you are in a dirty state. Change manually to ' - 'the branch that is being linted or stash your changes.' - % self.new_branch) + print('\nCould not change branch to %s. This is most probably ' + 'because you are in a dirty state. Change manually to ' + 'the branch that is being linted or stash your changes.' + % self.new_branch) sys.exit(1) def __exit__(self, exc_type, exc_val, exc_tb): @@ -272,8 +272,8 @@ def main(): collected_files = _collect_files_being_pushed(refs, remote) # only interfere if we actually have something to lint (prevent annoyances). if collected_files and _has_uncommitted_files(): - print ('Your repo is in a dirty state which prevents the linting from' - ' working.\nStash your changes or commit them.\n') + print('Your repo is in a dirty state which prevents the linting from' + ' working.\nStash your changes or commit them.\n') sys.exit(1) for branch, (modified_files, files_to_lint) in collected_files.iteritems(): with ChangedBranch(branch): diff --git a/tox.ini b/tox.ini index dc6ecaaab104..77d84f9982fa 100644 --- a/tox.ini +++ b/tox.ini @@ -1,2 +1,2 @@ [pycodestyle] -select=E231,E301,E302,E305 +select=E112,E113,E115,E116,E133,E201,E202,E203,E211,E221,E222,E225,E226,E228,E231,E241,E251,E271,E272,E275,E301,E302,E305,E714,W292,W293,W391 From c8c22c6931b1e03901ee956a69c000e8c707b547 Mon Sep 17 00:00:00 2001 From: Apurv-Bajaj Date: Tue, 15 May 2018 08:57:51 +0530 Subject: [PATCH 2/2] Review changes(1) --- core/controllers/editor_test.py | 2 +- core/tests/performance_framework/perf_domain.py | 16 ++++++++-------- scripts/backend_tests.py | 6 +++--- scripts/cut_release_branch.py | 6 +++--- scripts/install_third_party.py | 2 +- scripts/pre_commit_linter.py | 8 ++++---- scripts/pre_push_hook.py | 12 ++++++------ 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/core/controllers/editor_test.py b/core/controllers/editor_test.py index 29ffd7d9d95d..c40179d8339e 100644 --- a/core/controllers/editor_test.py +++ b/core/controllers/editor_test.py @@ -1659,7 +1659,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, diff --git a/core/tests/performance_framework/perf_domain.py b/core/tests/performance_framework/perf_domain.py index b361ce306882..9967552cd6c2 100644 --- a/core/tests/performance_framework/perf_domain.py +++ b/core/tests/performance_framework/perf_domain.py @@ -236,8 +236,8 @@ def print_details(self): """Helper function to print details for all the events.""" if self.page_session_stats: print 'Total number of requests: %d' % self.get_request_count() - print('Total page size in bytes: %d' - % self.get_total_page_size_bytes()) + print ('Total page size in bytes: %d' + % self.get_total_page_size_bytes()) else: print 'Page session stats are not available.' @@ -248,13 +248,13 @@ def print_details(self): print 'Ready start time: %d' % self.get_ready_start_time_millisecs() print 'Redirect time: %d' % self.get_redirect_time_millisecs() print 'Appcache time: %d' % self.get_appcache_time_millisecs() - print('Unload event time: %d' - % self.get_unload_event_time_millisecs()) + print ('Unload event time: %d' + % self.get_unload_event_time_millisecs()) print 'DNS query time: %d' % self.get_lookup_domain_time_millisecs() - print('TCP connection time: %d' - % self.get_connect_time_millisecs()) - print('Init domtree time: %d' - % self.get_init_dom_tree_time_millisecs()) + print ('TCP connection time: %d' + % self.get_connect_time_millisecs()) + print ('Init domtree time: %d' + % self.get_init_dom_tree_time_millisecs()) print 'Load event time: %d' % self.get_load_event_time_millisecs() else: print 'Page session timings are not available.' diff --git a/scripts/backend_tests.py b/scripts/backend_tests.py index 8d7bb40a90a1..dd42b1b3e8e2 100644 --- a/scripts/backend_tests.py +++ b/scripts/backend_tests.py @@ -328,10 +328,10 @@ def main(): r'Ran ([0-9]+) tests? in ([0-9\.]+)s', task.output) test_count = int(tests_run_regex_match.group(1)) test_time = float(tests_run_regex_match.group(2)) - print('SUCCESS %s: %d tests (%.1f secs)' % - (spec.test_target, test_count, test_time)) + print ('SUCCESS %s: %d tests (%.1f secs)' % + (spec.test_target, test_count, test_time)) except Exception: - print( + print ( 'An unexpected error occurred. ' 'Task output:\n%s' % task.output) diff --git a/scripts/cut_release_branch.py b/scripts/cut_release_branch.py index 12f2a6ed3b60..dcb2654fef0f 100644 --- a/scripts/cut_release_branch.py +++ b/scripts/cut_release_branch.py @@ -125,13 +125,13 @@ def _execute_branch_cut(): common.open_new_tab_in_browser_if_possible( 'https://github.com/oppia/oppia#oppia---') while True: - print( + print ( 'Please confirm: are Travis checks passing on develop? (y/n) ') answer = raw_input().lower() if answer in ['y', 'ye', 'yes']: break elif answer: - print( + print ( 'Tests should pass on develop before this script is run. ' 'Exiting.') sys.exit() @@ -163,7 +163,7 @@ def _execute_branch_cut(): subprocess.call(['git', 'push', remote_alias, NEW_BRANCH_NAME]) print '' - print( + print ( 'New release branch successfully cut. You are now on branch %s' % NEW_BRANCH_NAME) print 'Done!' diff --git a/scripts/install_third_party.py b/scripts/install_third_party.py index ef72b3d12548..52bd942eb841 100644 --- a/scripts/install_third_party.py +++ b/scripts/install_third_party.py @@ -227,7 +227,7 @@ def test_manifest_syntax(dependency_type, dependency_dict): print '------------------------------------------' print 'There is syntax error in this dependency' print dependency_dict - print( + print ( 'Only one of these keys pair must be used: "%s".' % str(optional_keys)) print 'Exiting' diff --git a/scripts/pre_commit_linter.py b/scripts/pre_commit_linter.py index d74448904b80..bdd6e5d825ae 100644 --- a/scripts/pre_commit_linter.py +++ b/scripts/pre_commit_linter.py @@ -486,8 +486,8 @@ def _get_all_files(): else: invalid_filepaths.append(f) if invalid_filepaths: - print('The following file(s) do not exist: %s\n' - 'Exiting.' % invalid_filepaths) + print ('The following file(s) do not exist: %s\n' + 'Exiting.' % invalid_filepaths) sys.exit(1) all_files = valid_filepaths else: @@ -592,7 +592,7 @@ def _check_newline_character(all_files): f.seek(-2, 2) if not (f.read(1) != '\n' and f.read(1) == '\n'): failed = True - print( + print ( '%s --> Please ensure that this file ends' 'with exactly one newline char.' % filename) total_error_count += 1 @@ -922,7 +922,7 @@ def _check_html_directive_name(all_files): if not directive_filename.endswith('_directive.html'): failed = True total_error_count += 1 - print( + print ( '%s --> Please ensure that this file ends' 'with _directive.html.' % directive_filename) if failed: diff --git a/scripts/pre_push_hook.py b/scripts/pre_push_hook.py index 287ef6de4f91..5b5321c0aa53 100755 --- a/scripts/pre_push_hook.py +++ b/scripts/pre_push_hook.py @@ -71,10 +71,10 @@ def __enter__(self): subprocess.check_output( ['git', 'checkout', self.new_branch, '--']) except subprocess.CalledProcessError: - print('\nCould not change branch to %s. This is most probably ' - 'because you are in a dirty state. Change manually to ' - 'the branch that is being linted or stash your changes.' - % self.new_branch) + print ('\nCould not change branch to %s. This is most probably ' + 'because you are in a dirty state. Change manually to ' + 'the branch that is being linted or stash your changes.' + % self.new_branch) sys.exit(1) def __exit__(self, exc_type, exc_val, exc_tb): @@ -272,8 +272,8 @@ def main(): collected_files = _collect_files_being_pushed(refs, remote) # only interfere if we actually have something to lint (prevent annoyances). if collected_files and _has_uncommitted_files(): - print('Your repo is in a dirty state which prevents the linting from' - ' working.\nStash your changes or commit them.\n') + print ('Your repo is in a dirty state which prevents the linting from' + ' working.\nStash your changes or commit them.\n') sys.exit(1) for branch, (modified_files, files_to_lint) in collected_files.iteritems(): with ChangedBranch(branch):