Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Mypy type annotations to domain folder files #14033

Closed
83 tasks done
sajalasati opened this issue Oct 9, 2021 · 242 comments
Closed
83 tasks done

Adding Mypy type annotations to domain folder files #14033

sajalasati opened this issue Oct 9, 2021 · 242 comments
Assignees
Labels
enhancement Label to indicate an issue is a feature/improvement

Comments

@sajalasati
Copy link
Contributor

sajalasati commented Oct 9, 2021

This issue will track the type annotation process of all the core/domain/*.py (and their test files). To get started, ask to be assigned to any unclaimed task. Tasks are claimed by adding the username of the claimer to the end of the task, e.g. @example.

Note: Main code file and its test file must be type-annotated together.

The general procedure to add type annotations is as follows:

  1. Start with the main code file first.
  2. Remove the file to be annotated from the mypy denylist (i.e. from NOT_FULLY_COVERED_FILES in scripts/run_mypy_checks.py). Run the mypy checks on the code file using python -m scripts.run_mypy_checks --files <path/to/file> to understand all the errors mypy is reporting for this file.
  3. Go over them one by one, and fix them by following the detailed instructions as mentioned in our wiki.
  4. PAY ATTENTION to our guidelines for using type ignores in the codebase.
  5. For testing the changes after annotating all the files, run python -m scripts.run_mypy_checks, which runs the type checks on the entire codebase, and ensure that no errors are being reported.
  6. If errors occur in other files due to you change make sure to also fix those other files.

Good examples of already annotated files in the codebase:

  1. Check out the core/storage folder.
  2. Ref PR for domain file type annotation: Fix part of #14033: Type annotate activity_domain file #14034

If you face problems in understanding a particular case while adding annotations, please go over the tips mentioned in the wiki once, and let us know if you are still stuck!

Tasks/files:

  • core/domain/action_registry.py@sahiljoster32
  • core/domain/activity_services.py@sahiljoster32
  • core/domain/auth_services.py @IamhappyXD
  • core/domain/blog_services.py @IamhappyXD
  • core/domain/calculation_registry.py@sahiljoster32
  • core/domain/classifier_services.py
  • core/domain/classroom_services.py @SD-13
  • core/domain/collection_services.py
  • core/domain/config_services.py@sahiljoster32
  • core/domain/cron_services.py @SD-13
  • core/domain/customization_args_util.py
  • core/domain/draft_upgrade_services.py
  • core/domain/email_manager.py
  • core/domain/email_services.py @jordyparker
  • core/domain/email_subscription_services.py@sahiljoster32
  • core/domain/event_services.py
  • core/domain/exp_fetchers.py
  • core/domain/exp_services.py
  • core/domain/expression_parser.py
  • core/domain/feedback_services.py
  • core/domain/fs_services.py @sahiljoster32
  • core/domain/html_cleaner.py @SD-13
  • core/domain/html_validation_service.py
  • core/domain/image_services.py @gopivaibhav
  • core/domain/image_validation_services.py @hsadia538
  • core/domain/improvements_services.py
  • core/domain/interaction_registry.py
  • core/domain/learner_goals_services.py
  • core/domain/learner_playlist_services.py
  • core/domain/learner_progress_services.py
  • core/domain/moderator_services.py
  • core/domain/object_registry.py
  • core/domain/opportunity_services.py
  • core/domain/platform_feature_services.py
  • core/domain/platform_parameter_list.py @Darsuu
  • core/domain/platform_parameter_list_test.py @Darsuu
  • core/domain/platform_parameter_registry.py @Darsuu
  • core/domain/playthrough_issue_registry.py @Darsuu
  • core/domain/question_fetchers.py @winnie368c
  • core/domain/question_services.py
  • core/domain/rating_services.py @IamhappyXD
  • core/domain/recommendations_services.py
  • core/domain/rights_manager.py
  • core/domain/role_services.py @gopivaibhav
  • core/domain/rte_component_registry.py @hsadia538
  • core/domain/rules_registry.py @hsadia538
  • core/domain/search_services.py
  • core/domain/skill_fetchers.py
  • core/domain/skill_services.py
  • core/domain/stats_services.py
  • core/domain/story_fetchers.py
  • core/domain/story_services.py
  • core/domain/subscription_services.py
  • core/domain/subtopic_page_services.py @ashish-patwal
  • core/domain/suggestion_registry.py
  • core/domain/suggestion_services.py
  • core/domain/summary_services.py
  • core/domain/takeout_service.py @jordyparker
  • core/domain/taskqueue_services.py @jordyparker
  • core/domain/topic_fetchers.py @SD-13
  • core/domain/topic_services.py @SD-13
  • core/domain/translatable_object_registry.py @ashish-patwal
  • core/domain/translation_fetchers.py @sahiljoster32
  • core/domain/translation_services.py @sahiljoster32
  • core/domain/user_query_services.py @SD-13
  • core/domain/user_services.py @SD-13
  • core/domain/visualization_registry.py' @SD-13
  • core/domain/voiceover_services.py @SD-13
  • core/domain/wipeout_service.py @SD-13
  • core/domain/change_domain.py @div-yam
  • core/domain/collection_domain.py @sahiljoster32
  • core/domain/exp_domain.py @Yavnikaa
  • core/domain/fs_domain.py @PranshuSrivastava
  • core/domain/param_domain.py @asmit2952 (attempted in Mypy annotation 3 files #14302)
  • core/domain/platform_parameter_domain.py @asmit2952 (attempted in Mypy annotation 3 files #14302)
  • core/domain/question_domain.py @asmit2952 (attempted in Mypy annotation 3 files #14302)
  • core/domain/skill_domain.py @dhruvshrivastava
  • core/domain/state_domain.py @dhruvshrivastava
  • core/domain/stats_domain.py @winnie368c
  • core/domain/story_domain.py @Sangkyun-Kim15
  • core/domain/subtopic_page_domain.py @ashish-patwal
  • core/domain/user_domain.py@sahiljoster32
  • core/domain/user_query_domain.py @ashish-patwal
@kingjuno
Copy link
Contributor

@sajalasati Assign me the following
core/domain/auth_domain.py
core/domain/blog_domain.py
core/domain/caching_domain.py

@sajalasati
Copy link
Contributor Author

@sajalasati Assign me the following core/domain/auth_domain.py core/domain/blog_domain.py core/domain/caching_domain.py

@kingjuno I've assigned you now!

@kingjuno kingjuno self-assigned this Oct 12, 2021
@vashuteotia123
Copy link
Contributor

@sajalasati Can I get any three files assigned to me?

@sajalasati
Copy link
Contributor Author

@sajalasati Can I get any three files assigned to me?

Done, assigned you the next 3 in order.

@aniketh-varma
Copy link

Hi @sajalasati, I would like to work on this. Please assign me a file.

@sajalasati
Copy link
Contributor Author

Hi @sajalasati, I would like to work on this. Please assign me a file.

Done!

@vashuteotia123
Copy link
Contributor

image
@sajalasati Can you just tell me what will be the annotation for this?

@ShivamJhaa
Copy link
Contributor

Hey @sajalasati Can you please assign me
core/domain/config_domain.py
core/domain/exp_domain.py
Thanks

kingjuno added a commit to kingjuno/oppia that referenced this issue Oct 12, 2021
@sajalasati
Copy link
Contributor Author

@sajalasati Can you just tell me what will be the annotation for this?

@vashuteotia123 We could use List[Dict[str,Union(int,List[str])]] for this case.

Just a note for future, it would be helpful if you can provide a link to the code (file) where you have doubts, and possibly your guessed annotation too, so that we can help you understand what (and why) that annotation should be. I hope this helps :)

@sajalasati
Copy link
Contributor Author

sajalasati commented Oct 13, 2021

Hey @sajalasati Can you please assign me core/domain/config_domain.py core/domain/exp_domain.py Thanks

@ShivamJhaa Done!

@vashuteotia123
Copy link
Contributor

@sajalasati Can you just tell me what will be the annotation for this?

@vashuteotia123 We could use List[Dict[str,Union(int,List[str])]] for this case.

Just a note for future, it would be helpful if you can provide a link to the code (file) where you have doubts, and possibly your guessed annotation too, so that we can help you understand what (and why) that annotation should be. I hope this helps :)

@sajalasati I did somewhat the same, I didn't know about Union, will understand the working of it and implement it.
Thanks.

@sajalasati
Copy link
Contributor Author

@sajalasati Can you just tell me what will be the annotation for this?

@vashuteotia123 We could use List[Dict[str,Union(int,List[str])]] for this case.
Just a note for future, it would be helpful if you can provide a link to the code (file) where you have doubts, and possibly your guessed annotation too, so that we can help you understand what (and why) that annotation should be. I hope this helps :)

@sajalasati I did somewhat the same, I didn't know about Union, will understand the working of it and implement it. Thanks.

@vashuteotia123 No issues, just check for it in the codebase, you'll find plenty of examples.

@ShivamJhaa
Copy link
Contributor

Hey @sajalasati Can you help me in determining the type annotation of these two
image
in core/domain/config_domain.py.
Thanks

@lheureuxe13
Copy link
Contributor

Hi @sajalasati, please assign me 3 where needed. Thanks

kingjuno added a commit to kingjuno/oppia that referenced this issue Oct 14, 2021
nithusha21 pushed a commit that referenced this issue Oct 14, 2021
@ShivamJhaa
Copy link
Contributor

@vojtechjelinek @sajalasati What will be the annotation for this?
image
Thanks

@megamayoy
Copy link
Contributor

@sajalasati I wanna contribute during the Hacktoberfest. Will you please assign me 2 files?

@vojtechjelinek
Copy link
Member

Hey @sajalasati Can you help me in determining the type annotation of these two image in core/domain/config_domain.py. Thanks

These shouldn't need any typing.

@vojtechjelinek
Copy link
Member

@megamayoy @lheureuxe13 Done!

@sahiljoster32
Copy link
Contributor

Hi @hsadia538 ,
Sorry to say but this issue from now is not available for new contributors. Because the files mentioned in this issue is going to be covered under the GSoC project Make backend code typed.

But still, you can find other beginner-friendly issues by searching the good first issue label in the issues tab or you can click here for beginner-friendly issues

aks681 pushed a commit that referenced this issue Jun 24, 2022
…ain` folder. -- M1.2 (#15580)

* added auth_services

* added task_queue

* added subscription_services

* added recommendations

* added playthrough

* added learner goal

* added interaction registry

* added improvements regisrty

* nits

* nits

* fixes

* added change_domain

* added custom_utils

* nits

* nits

* added changes

* some refactoring

* nits

* reverted get()

* removed comment

* comment nits

* minors

* nits

* added fixes

* nits

* nits

* added todo

* lint

* removed cast

* fixed backend test

* fixing backend test try -1

* removeed last_updated

* added test file for coverage

* nits

* refactored

* nits

* removed test

* removed

* reverted things

* nits

* tsting - 1

* reverted and cleaned every thing

* nits fixed

* mypy fixes

* added comment

* added todos
iamprayush pushed a commit that referenced this issue Jul 3, 2022
sahiljoster32 added a commit that referenced this issue Jul 3, 2022
…ain` folder. -- M1.3 (#15596)

* added platform_param

* added rules_registry

* added moderator_services

* added learner_playlist

* added parameter_registry

* added rte_registry

* fixed mypy tests

* fixed backend

* checking backend failures

* added image validation

* some nits

* added lint fix

* requested changes

* nits

* added fixes

* nits

* added todos

* nits

* requested changes

* added changes

* fixed comment

* minor nits

* added comments

* nits

* fixes

* added changes

* reverted things

* removed optinals

* nits

* added raises

* added fixes

* added dash dash
@swethasekhar
Copy link
Contributor

swethasekhar commented Jul 6, 2022

Hi Team ,
I get the below errors for stats_services.py file
The error is
Value of type variable "SELF_BASE_MODEL" of "update_timestamps_multi" of "BaseModel" cannot be "Optional[PlaythroughModel]" [type-var]

Value of type variable "SELF_BASE_MODEL" of "put_multi" of "BaseModel" cannot be "Optional[PlaythroughModel]" [type-var]
Screen Shot 2022-06-22 at 10 25 08 PM

Thanks for your help !

@aasiffaizal
Copy link
Contributor

@swethasekhar
You might need to overload the function in PlaythroughModel for those functions and specify the types of parameter and return type (if applicable).
You can refer to the code base here we have used it.

aasiffaizal pushed a commit that referenced this issue Jul 7, 2022
…in folder. -- M1.5 (#15640)

* added platform_parameter_list

* added classifer_services.py

* added questions fetchers

* added changes

* nits

* added exp_fetchers

* fixed backend

* reverted changes

* added fixes

* added fixes

* added new changes for conflict

* added nits

* reomved Any

* added things

* minor changes

* nits

* more fix

* nits

* nits

* comment

* reverted some changes due to backend failuer

* changed type to bytes

* added changes

* removed . thing

* added assert

* nits

* removed if cluase

* reverted to check backend test
@Darsuu
Copy link
Contributor

Darsuu commented Jul 10, 2022

@vojtechjelinek could you unassign me from my files? Thank you

@vojtechjelinek
Copy link
Member

@Darsuu Sorry, this issue is no longer available for new contributors. Please check issues with "Good first issue" label.

sahiljoster32 pushed a commit that referenced this issue Jul 17, 2022
* Added mypy type annotations stats_domain.py

* Added mypy type annotations to stats_domain.py

* Adding mypy type annotations to stats_domain.py

* Adding mypy type annotations to stats_domain.py

* Adding mypy type annotations to stats_domain.py

* Adding mypy type annotations to stats_domain.py

* Adding mypy type annotations to stats_domain.py

* Addressing review comments

* Addressing review comments

* Addressing review comments

* Addressed review comments

* Addressed review comments

* Addressed review comments

* Addressed review comments

* Addressing review comments

* Addressed review comments

* Addressed review comments

* Addressing review comments

* Adding mypy type annotations to question_fetchers.py

* addressing review comments

* fixing stats_domain_test.py tests

* Addressing review comments

* Addressing review comments

* Addressing review comments

* fixing any comments

* fixing any

* fixing any

* fixing backend test failures

* fixing backend test

* fixing backend tests

* fixing backend test

* fixing backend tests

* fixing keyerror backend tests

* fixing keyerror tests

* fixing backend test coverage

* resolving merge conflicts

* adding mypy type annotations for new methods

* adding mypy type annotations to validate_aggregated_stats_dict method

* fixing customization_args type

* addressing review comments

* fix convert actionv1 to v2dict test

* fix convert action dict test

* resolve merge conflicts

* fixed action dict conversion test

* fixed action dict conversion test

* addressing review comments

* lint

* import final
sahiljoster32 added a commit that referenced this issue Aug 10, 2022
…in folder. -- M1.8 (#15762)

* added collection services

* added html_validation

* added opportunity services

* added question services

* added fixes

* added event services

* nits

* added changes

* added optional changes

* nits

* added changes

* added changes

* added changes

* reverted optional for story

* added changes

* changed base type

* added comments

* added changes

* added nits fixes

* changes

* changes

* added changes

* added fixes for backend

* addec hanges

* added changes

* nits
sahiljoster32 added a commit that referenced this issue Aug 11, 2022
…in folder. -- M1.9 (#15801)

* added voice_services

* added story_services

* added half question domain

* half test file and full main file

* added question domain

* added changes for suggestion services

* added full test file

* added changes

* added suggestion_reigstry

* minor fixes

* more minor fixes

* added summary_services

* added expression_parser.py

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* added changes

* removed merge conflict issues

* nits

* nit

* fixes

* nits
sahiljoster32 added a commit that referenced this issue Aug 12, 2022
…in folder. -- M1.7 (#15693)

* added state_domain for support

* added draft_upgrade_service

* added feedback services

* added backend fixes for draf_services

* pending test file

* added right_manager's test file

* removed assertion because it failing backend tests

* added exp_domian_test half

* added full exp_domain_test

* added skill domain and take self-pass before assign

* nits and take pass on tommorrow morning

* added changes

* added chenages

* nits

* added changes

* minor nits:

* added changes -2

* added changes and going to sleep

* added changes

* nits

* nits again

* nits

* added docs

* added assertions

* added assertion -2

* added returns

* added explanations

* added changes

* removed dead code

* reverted coverage

* nits

* nits

* added changes

* added if clause

* added changes

* nits

* added small changes

* added tests

* nits

* added nits

* added changes

* added changes

* added changes

* added failing test

* added changes

* added changes

* added changes

* added changes

* fixed backend tests

* minor change

* added changes

* nits

* added changes

* nits

* added changes - after merge conflict

* added changes

* nits

* added changes

* nits
sahiljoster32 added a commit that referenced this issue Aug 28, 2022
…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
@kevintab95 kevintab95 added the enhancement Label to indicate an issue is a feature/improvement label Aug 30, 2022
Shivkant-Chauhan pushed a commit to Shivkant-Chauhan/oppia that referenced this issue Sep 1, 2022
… domain folder. -- M1.10 (oppia#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
@sahiljoster32
Copy link
Contributor

@seanlip @aasiffaizal -- I think we can close this issue as we have covered all the files of the code/domain directory, Thanks.

@kevintab95 -- One question why Improvement label is added ?, I think all files of code/domain are covered.

@kevintab95
Copy link
Member

kevintab95 commented Sep 6, 2022

Considering this is an infra / maintenance related issue, I thought it was relevant to tag as "improvement" (as opposed to a "bug".)

@seanlip
Copy link
Member

seanlip commented Sep 12, 2022

@sahiljoster32 Sounds good, closing it. Thanks!

@seanlip seanlip closed this as completed Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Label to indicate an issue is a feature/improvement
Projects
Archived in project
Development

No branches or pull requests