Accessing custom hook type changed to dict.get()

[lorinczszabolcs]

Since hook_cfg is a dictionary, its 'type' member can not be accessed with the current dot notation (hook_cfg.type), rather it has to be accessed either with hook_cfg['type'], but that would assume the 'type' key exists, so a better alternative is to access it with hook_cfg.get('type', None). Fixes #396

Motivation

Adding custom hooks is not possible currently, since their type is checked with hook_cfg.type instead of hook_cfg.get('type, None). This raises an error, since dot notation does not work for accessing values of dictionaries.

Modification

hook_cfg.type changed to hook_cfg.get('type, None).

BC-breaking (Optional)

Shouldn't break backward-compatibility.

Checklist

Before PR:

• [x] Pre-commit or other linting tools are used to fix the potential lint issues.
• [x] Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests.
• [x] The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
• [x] The documentation has been modified accordingly, like docstring or example tutorials.

After PR:

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

[codecov[bot]]

Codecov Report

Merging #403 (0a9b729) into master (abcd43b) will not change coverage. The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #403   +/-   ##
=======================================
  Coverage   72.47%   72.47%           
=======================================
  Files         121      121           
  Lines        4759     4759           
  Branches      763      763           
=======================================
  Hits         3449     3449           
  Misses       1151     1151           
  Partials      159      159           

Flag	Coverage Δ
unittests	72.47% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
mmselfsup/apis/train.py	47.77% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[fangyixiao18]

Hi, thanks for your suggestion and PR. I think the description is not very precise. If we write hook_cfg in config files, mmcv will parse the config and we could access the member with dot notation. https://github.com/open-mmlab/mmcv/blob/master/mmcv/utils/config.py#L72 However, your modification is great, we didn't think of the python dictionary situation.

LGTM.

[lorinczszabolcs]

Hi, thanks for your suggestion and PR. I think the description is not very precise. If we write hook_cfg in config files, mmcv will parse the config and we could access the member with dot notation. https://github.com/open-mmlab/mmcv/blob/master/mmcv/utils/config.py#L72 However, your modification is great, we didn't think of the python dictionary situation.

LGTM.

Ah, I see, thanks for the clarification!

[lorinczszabolcs]

Sorry, I pushed commits by mistake to the original branch, and didn't know it will also appear here. The commits I added are trying to fix #400, I don't know if it's ok to also add that here, or how should we proceed.

[fangyixiao18]

Sorry, I pushed commits by mistake to the original branch, and didn't know it will also appear here. The commits I added are trying to fix #400, I don't know if it's ok to also add that here, or how should we proceed.

It is better to separate the modification to 2 PRs. You need to checkout a branch before you modify the codes. And 2 PRs are supposed to be from 2 branches. Thanks!

[lorinczszabolcs]

Sorry, I pushed commits by mistake to the original branch, and didn't know it will also appear here. The commits I added are trying to fix #400, I don't know if it's ok to also add that here, or how should we proceed.

It is better to separate the modification to 2 PRs. You need to checkout a branch before you modify the codes. And 2 PRs are supposed to be from 2 branches. Thanks!

Will close this pull request and reopen two new ones with the modifications on separate branches. Sorry for the confusion. See #405 and #406