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

Add type hints for runner/base_runner #2003

Merged

Conversation

ytzhao
Copy link
Contributor

@ytzhao ytzhao commented May 26, 2022

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

Add type hints for runner/base_runner.

Modification

  • mmcv/runner/base_runner.py
  • mmcv/runner/checkpoint.py
  • mmcv/runner/epoch_based_runner.py
  • mmcv/runner/hooks/lr_updater.py
  • mmcv/runner/iter_based_runner.py

BC-breaking (Optional)

No

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with some of those projects, like MMDet or MMCls.
  • CLA has been signed and all committers have signed the CLA in this PR.

@HAOCHENYE
Copy link
Collaborator

Hi~ Is there any progress?

@ytzhao ytzhao force-pushed the ytzhao/type_hints_runner_base_runner branch from 2820e7d to 0d0fee6 Compare May 31, 2022 06:05
@ytzhao
Copy link
Contributor Author

ytzhao commented May 31, 2022

Hi~ Is there any progress?

Hi @HAOCHENYE already updated, pls take a review, thanks ;-)

@zhouzaida
Copy link
Collaborator

Hi~ Is there any progress?

Hi @HAOCHENYE already updated, pls take a review, thanks ;-)

Hi, thanks for your contributions. We can also add type hints for epoch_based_runner.py and iter_based_runner.py at this PR.

@HAOCHENYE
Copy link
Collaborator

Hi~ @ytzhao, Is there any progress?

@ytzhao ytzhao force-pushed the ytzhao/type_hints_runner_base_runner branch 4 times, most recently from c74b6c2 to ff9af0b Compare June 9, 2022 00:07
@HAOCHENYE
Copy link
Collaborator

@ytzhao Hi~ The lint seems failed.

@ytzhao ytzhao force-pushed the ytzhao/type_hints_runner_base_runner branch from ff9af0b to 9cd32dd Compare June 10, 2022 00:38
@ytzhao
Copy link
Contributor Author

ytzhao commented Jun 12, 2022

@HAOCHENYE Hi, the unit_tests job is in progress for a long time, it started 2d ish ago, weird.

@zhouzaida
Copy link
Collaborator

@HAOCHENYE Hi, the unit_tests job is in progress for a long time, it started 2d ish ago, weird.

Hi, that is ok. That unit test needs to be triggered by us manually.

@ytzhao ytzhao force-pushed the ytzhao/type_hints_runner_base_runner branch from 9cd32dd to e778a0b Compare June 13, 2022 16:10
@ytzhao
Copy link
Contributor Author

ytzhao commented Jun 15, 2022

@HAOCHENYE Hi, the unit_tests job is in progress for a long time, it started 2d ish ago, weird.

Hi, that is ok. That unit test needs to be triggered by us manually.

@zhouzaida @HAOCHENYE Hi, please check the latest updated, thanks

@zhouzaida
Copy link
Collaborator

@HAOCHENYE Hi, the unit_tests job is in progress for a long time, it started 2d ish ago, weird.

Hi, that is ok. That unit test needs to be triggered by us manually.

@zhouzaida @HAOCHENYE Hi, please check the latest updated, thanks

Okay

@zhouzaida zhouzaida merged commit 3dd2a21 into open-mmlab:master Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants