Fix part of #14033: Added Mypy type annotations to some files of doma…

…in folder. -- M1.10 (#15827) * added wipeout_service * added learner_progress * added skill_services * added exp_services * added changes * added backend fixes * added fixes * added changes * minor fix * added state_domain * added backend fixes * fixes backend tests and coverage * nits * added changes * added changes * added changes * added changes * nits * added changes * nits * added tests * added changes * added changes * added changes * added changes * nits * added changes * changes * added changes * added changes * resolved merge conflict * added changes * added changes * nits * added changes * fixed merged conflict * added changes * added changes * added changes * added changes * fixed minore changes * resovled merge conflict * added changes * added changes * nits * added changes * added changes * changes * added changes * nits * nits -- found
oppia · Aug 28, 2022 · bb0ecf6 · bb0ecf6
1 parent dfd35dc
commit bb0ecf6
Show file tree

Hide file tree

Showing 83 changed files with 5,430 additions and 2,740 deletions.
diff --git a/core/controllers/topic_viewer_test.py b/core/controllers/topic_viewer_test.py
@@ -235,7 +235,6 @@ def test_get_with_user_logged_in(self):
                     self.skill_id_2: 0.5
                 },
                 'skill_descriptions': {
-                    self.skill_id_1: None,
                     self.skill_id_2: 'Skill Description 2'
                 },
                 'practice_tab_is_displayed': False
@@ -338,8 +337,7 @@ def test_get_with_five_or_more_questions(self):
                 self.skill_id_2: None
             },
             'skill_descriptions': {
-                self.skill_id_1: 'Skill Description 1',
-                self.skill_id_2: None
+                self.skill_id_1: 'Skill Description 1'
             },
             'practice_tab_is_displayed': True
         }
@@ -392,7 +390,6 @@ def test_get_with_twenty_or_more_questions(self):
             },
             'skill_descriptions': {
                 self.skill_id_1: 'Skill Description 1',
-                self.skill_id_2: None
             },
             'practice_tab_is_displayed': True
         }

diff --git a/core/domain/activity_services_test.py b/core/domain/activity_services_test.py
@@ -136,7 +136,7 @@ def test_deleted_activity_is_removed_from_featured_list(self) -> None:
                 self._create_collection_reference(self.COL_ID_2)])
 
         # Deleting an unfeatured activity does not affect the featured list.
-        exp_services.delete_exploration(self.owner_id, self.EXP_ID_1) # type: ignore[no-untyped-call]
+        exp_services.delete_exploration(self.owner_id, self.EXP_ID_1)
         self._compare_lists(
             activity_services.get_featured_activity_references(), [
                 self._create_exploration_reference(self.EXP_ID_0),
@@ -147,7 +147,7 @@ def test_deleted_activity_is_removed_from_featured_list(self) -> None:
         self._compare_lists(
             activity_services.get_featured_activity_references(), [
                 self._create_exploration_reference(self.EXP_ID_0)])
-        exp_services.delete_exploration(self.owner_id, self.EXP_ID_0) # type: ignore[no-untyped-call]
+        exp_services.delete_exploration(self.owner_id, self.EXP_ID_0)
         self._compare_lists(
             activity_services.get_featured_activity_references(), [])
 
@@ -166,7 +166,7 @@ def test_deleted_activity_is_removed_from_featured_list_multiple(
             activity_services.get_featured_activity_references(),
             exploration_references)
 
-        exp_services.delete_explorations( # type: ignore[no-untyped-call]
+        exp_services.delete_explorations(
             self.owner_id, [self.EXP_ID_0, self.EXP_ID_1])
         self._compare_lists(
             activity_services.get_featured_activity_references(), [])

diff --git a/core/domain/change_domain.py b/core/domain/change_domain.py
@@ -33,6 +33,7 @@
     from core.domain import param_domain
     from core.domain import platform_parameter_domain
     from core.domain import question_domain
+    from core.domain import skill_domain
     from core.domain import state_domain
 
     # After importing modules under the `if MYPY` clause they are not
@@ -48,13 +49,14 @@
         bool,
         float,
         int,
-        float,
         None,
         List[str],
         Dict[str, Any],
         List[Dict[str, Any]],
         List[param_domain.ParamChangeDict],
         List[state_domain.AnswerGroupDict],
+        List[state_domain.HintDict],
+        List[skill_domain.WorkedExampleDict],
         List[platform_parameter_domain.PlatformParameterRuleDict],
         question_domain.QuestionDict,
         state_domain.AnswerGroupDict,

diff --git a/core/domain/classifier_domain.py b/core/domain/classifier_domain.py
@@ -21,14 +21,11 @@
 
 from core import feconf
 from core import utils
+from core.domain import state_domain
 
-from typing import Dict, List, Union
+from typing import Dict, List
 from typing_extensions import TypedDict
 
-TrainingDataType = Union[
-    Dict[str, Union[int, List[str]]], List[Dict[str, Union[int, List[str]]]]
-]
-
 
 class ClassifierTrainingJobDict(TypedDict):
     """Dictionary that represents ClassifierTrainingJob."""
@@ -41,7 +38,7 @@ class ClassifierTrainingJobDict(TypedDict):
     next_scheduled_check_time: datetime.datetime
     state_name: str
     status: str
-    training_data: TrainingDataType
+    training_data: List[state_domain.TrainingDataDict]
     algorithm_version: int
 
 
@@ -103,7 +100,7 @@ def __init__(
         next_scheduled_check_time: datetime.datetime,
         state_name: str,
         status: str,
-        training_data: TrainingDataType,
+        training_data: List[state_domain.TrainingDataDict],
         algorithm_version: int
     ) -> None:
         """Constructs a ClassifierTrainingJob domain object.
@@ -233,7 +230,7 @@ def status(self) -> str:
         return self._status
 
     @property
-    def training_data(self) -> TrainingDataType:
+    def training_data(self) -> List[state_domain.TrainingDataDict]:
         """Returns the training data used for training the classifier.
 
         Returns:

diff --git a/core/domain/classifier_domain_test.py b/core/domain/classifier_domain_test.py
@@ -25,16 +25,19 @@
 from core import feconf
 from core import utils
 from core.domain import classifier_domain
+from core.domain import state_domain
 from core.tests import test_utils
 
+from typing import List
+
 
 class ClassifierTrainingJobDomainTests(test_utils.GenericTestBase):
     """Test the ClassifierTrainingJob domain."""
 
     def setUp(self) -> None:
         super().setUp()
 
-        self.training_data: classifier_domain.TrainingDataType = [
+        self.training_data: List[state_domain.TrainingDataDict] = [
             {
                 'answer_group_index': 1,
                 'answers': ['a1', 'a2']
@@ -82,6 +85,16 @@ def _get_training_job_from_dict(
         return training_job
 
     def test_to_dict(self) -> None:
+        expected_training_data: List[state_domain.TrainingDataDict] = [
+                {
+                    'answer_group_index': 1,
+                    'answers': ['a1', 'a2']
+                },
+                {
+                    'answer_group_index': 2,
+                    'answers': ['a2', 'a3']
+                }
+            ]
         expected_training_job_dict: (
             classifier_domain.ClassifierTrainingJobDict
         ) = {
@@ -95,16 +108,7 @@ def test_to_dict(self) -> None:
                     '2017-08-11 12:42:31', '%Y-%m-%d %H:%M:%S'),
             'state_name': 'a state name',
             'status': 'NEW',
-            'training_data': [
-                {
-                    'answer_group_index': 1,
-                    'answers': ['a1', 'a2']
-                },
-                {
-                    'answer_group_index': 2,
-                    'answers': ['a2', 'a3']
-                }
-            ],
+            'training_data': expected_training_data,
             'algorithm_version': 1
         }
         observed_training_job = self._get_training_job_from_dict(
@@ -131,9 +135,12 @@ def test_validation_interaction_id(self) -> None:
             utils.ValidationError, 'Invalid interaction id'):
             training_job.validate()
 
+    # TODO(#13059): After we fully type the codebase we plan to get
+    # rid of the tests that intentionally test wrong inputs that we
+    # can normally catch by typing.
     def test_validation_training_data_without_answer_group_index(self) -> None:
         self.training_job_dict['training_data'] = [
-            {
+            {  # type: ignore[typeddict-item]
                 'answers': ['a1', 'a2']
             }
         ]
@@ -144,9 +151,12 @@ def test_validation_training_data_without_answer_group_index(self) -> None:
             'list item'):
             training_job.validate()
 
+    # TODO(#13059): After we fully type the codebase we plan to get
+    # rid of the tests that intentionally test wrong inputs that we
+    # can normally catch by typing.
     def test_validation_training_data_without_answers(self) -> None:
         self.training_job_dict['training_data'] = [
-            {
+            {  # type: ignore[typeddict-item]
                 'answer_group_index': 1
             }
         ]
@@ -174,8 +184,11 @@ def test_validation_with_invalid_algorithm_id(self) -> None:
             utils.ValidationError, 'Invalid algorithm id'):
             training_job.validate()
 
+    # TODO(#13059): After we fully type the codebase we plan to get
+    # rid of the tests that intentionally test wrong inputs that we
+    # can normally catch by typing.
     def test_validation_with_invalid_training_data(self) -> None:
-        self.training_job_dict['training_data'] = {}
+        self.training_job_dict['training_data'] = {}  # type: ignore[arg-type]
         training_job = self._get_training_job_from_dict(self.training_job_dict)
         with self.assertRaisesRegex(
             utils.ValidationError, 'Expected training_data to be a list'):

diff --git a/core/domain/classifier_services.py b/core/domain/classifier_services.py
@@ -28,9 +28,11 @@
 from core.domain import exp_domain
 from core.domain import exp_fetchers
 from core.domain import fs_services
+from core.domain import state_domain
 from core.platform import models
 
 from typing import Dict, List, Optional, Sequence
+from typing_extensions import TypedDict
 
 MYPY = False
 if MYPY: # pragma: no cover
@@ -97,6 +99,20 @@ def verify_signature(
     return True
 
 
+class JobInfoDict(TypedDict):
+    """Type for the job info dictionary."""
+
+    algorithm_id: str
+    interaction_id: str
+    exp_id: str
+    exp_version: int
+    next_scheduled_check_time: datetime.datetime
+    state_name: str
+    training_data: List[state_domain.TrainingDataDict]
+    status: str
+    algorithm_version: int
+
+
 def handle_trainable_states(
     exploration: exp_domain.Exploration,
     state_names: List[str]
@@ -113,12 +129,12 @@ def handle_trainable_states(
     Raises:
         Exception. No classifier algorithm found for the given interaction id.
     """
-    job_dicts_list = []
+    job_dicts_list: List[JobInfoDict] = []
     exp_id = exploration.id
     exp_version = exploration.version
     for state_name in state_names:
         state = exploration.states[state_name]
-        training_data = state.get_training_data()  # type: ignore[no-untyped-call]
+        training_data = state.get_training_data()
         interaction_id = state.interaction.id
         if interaction_id not in feconf.INTERACTION_CLASSIFIER_MAPPING:
             raise Exception(
@@ -618,11 +634,11 @@ def migrate_state_training_jobs(
         set(state_training_jobs_mapping.algorithm_ids_to_job_ids.keys()))
 
     if len(algorithm_ids_to_add) > 0:
-        job_dicts = []
+        job_dicts: List[JobInfoDict] = []
 
         for algorithm_id in algorithm_ids_to_add:
             next_scheduled_check_time = datetime.datetime.utcnow()
-            training_data = exploration.states[state_name].get_training_data()  # type: ignore[no-untyped-call]
+            training_data = exploration.states[state_name].get_training_data()
 
             classifier_domain.ClassifierTrainingJob(
                 'job_id_dummy', algorithm_id, interaction_id, exp_id,

diff --git a/core/domain/classifier_services_test.py b/core/domain/classifier_services_test.py
@@ -35,7 +35,7 @@
 from core.tests import test_utils
 from proto_files import text_classifier_pb2
 
-from typing import Dict, List
+from typing import Dict, List, Tuple
 
 MYPY = False
 if MYPY: # pragma: no cover
@@ -65,9 +65,9 @@ def _init_classify_inputs(self, exploration_id: str) -> None:
         test_exp_filepath = os.path.join(
             feconf.TESTS_DATA_DIR, 'string_classifier_test.yaml')
         yaml_content = utils.get_file_contents(test_exp_filepath)
-        assets_list: List[str] = []
+        assets_list: List[Tuple[str, bytes]] = []
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.save_new_exploration_from_yaml_and_assets(  # type: ignore[no-untyped-call]
+            exp_services.save_new_exploration_from_yaml_and_assets(
                 feconf.SYSTEM_COMMITTER_ID, yaml_content, exploration_id,
                 assets_list)
 
@@ -140,7 +140,7 @@ def test_creation_of_jobs_and_mappings(self) -> None:
             'new_value': state.recorded_voiceovers.to_dict()
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should be two jobs and two mappings in the data store now.
@@ -158,7 +158,7 @@ def test_creation_of_jobs_and_mappings(self) -> None:
             'new_value': 'New title'
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should be two jobs and three mappings in the data store now.
@@ -179,7 +179,7 @@ def test_creation_of_jobs_and_mappings(self) -> None:
             'new_state_name': 'Home3'
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should still be only two jobs and four mappings in the data
@@ -229,7 +229,7 @@ def test_that_models_are_recreated_if_not_available(self) -> None:
             'new_value': state.recorded_voiceovers.to_dict()
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should be two jobs and two mappings in the data store now.
@@ -247,7 +247,7 @@ def test_that_models_are_recreated_if_not_available(self) -> None:
             'new_value': 'New title'
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', False):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should be two jobs and two mappings in the data store now.
@@ -267,7 +267,7 @@ def test_that_models_are_recreated_if_not_available(self) -> None:
             'new_value': 'New title'
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         # There should be three jobs and three mappings in the data store now.
@@ -823,6 +823,7 @@ def test_reverted_exploration_maintains_classifier_model_mapping(
         exploration = exp_fetchers.get_exploration_by_id(self.exp_id)
         interaction_id = exploration.states[
             state_name].interaction.id
+        # Ruling out the possibility of None for mypy type checking.
         assert interaction_id is not None
         algorithm_id = feconf.INTERACTION_CLASSIFIER_MAPPING[
             interaction_id]['algorithm_id']
@@ -834,7 +835,7 @@ def test_reverted_exploration_maintains_classifier_model_mapping(
             'new_value': 'A new title'
         })]
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.update_exploration(  # type: ignore[no-untyped-call]
+            exp_services.update_exploration(
                 feconf.SYSTEM_COMMITTER_ID, self.exp_id, change_list, '')
 
         current_exploration = exp_fetchers.get_exploration_by_id(self.exp_id)
@@ -847,7 +848,7 @@ def test_reverted_exploration_maintains_classifier_model_mapping(
         old_job_id = old_job.job_id
         # Revert the exploration.
         with self.swap(feconf, 'ENABLE_ML_CLASSIFIERS', True):
-            exp_services.revert_exploration(  # type: ignore[no-untyped-call]
+            exp_services.revert_exploration(
                 feconf.SYSTEM_COMMITTER_ID,
                 self.exp_id,
                 current_exploration.version,
@@ -884,7 +885,7 @@ def test_migrate_state_training_jobs_with_invalid_interaction_id(
             'property_name': exp_domain.STATE_PROPERTY_INTERACTION_ID,
             'new_value': None
         })]
-        exp_services.update_exploration(  # type: ignore[no-untyped-call]
+        exp_services.update_exploration(
             feconf.SYSTEM_COMMITTER_ID, exploration.id, change_list, '')
         state_training_jobs_mapping = (
             classifier_domain.StateTrainingJobsMapping('44', 2, 'New state', {})