Add tests/utilities/test_parsing.py

[akihironitta]

What does this PR do?

Fixes #4400.

TODO:

Add tests for:

• str_to_bool_or_str
• str_to_bool
• is_picklable
• clean_namespace
• parse_class_init_keys
• get_init_args <- NEED HELP Thank you @ethanwharris!
• collect_init_args <- NEED HELP Thank you @ethanwharris!
• flatten_dict
• AttributeDict

Before submitting

• Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
• Did you read the contributor guideline, Pull Request section?
• Did you make sure your PR does only one thing, instead of bundling different changes together? Otherwise, we ask you to create a separate PR for every change.
• Did you make sure to update the documentation with your changes?
• Did you write any new necessary tests?
• Did you verify new and existing tests pass locally with your changes?
• If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

• Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

[akihironitta]

I believe pytorch_lightning.utilities.parsing.lightning_hasattr should return either True or False as Python built-in hasattr returns so. However, with the current implementation, lightning_hasattr returns non-bool object in some cases.
https://github.com/PyTorchLightning/pytorch-lightning/blob/ebc1b23fa38d54e9805aa4356867369f064c7031/pytorch_lightning/utilities/parsing.py#L177

[akihironitta]

Also, when model doesn't have attribute, lightning_getattr(model, attribute) should raise AttributeError rather than ValueError.
https://github.com/PyTorchLightning/pytorch-lightning/blob/ebc1b23fa38d54e9805aa4356867369f064c7031/pytorch_lightning/utilities/parsing.py#L219

[pep8speaks]

Hello @akihironitta! Thanks for updating this PR.

• In the file tests/utilities/test_parsing.py:

Line 33:1: E731 do not assign a lambda expression, use a def

Comment last updated at 2021-03-02 01:16:20 UTC

[akihironitta]

Python built-in has_attr is implemented by calling getattr(object, name) and seeing whether it raises an AttributeError or not, as described in docs. Let's follow the implementation here as well so that we can simplify lightning_hasattr.

[codecov]

Codecov Report

Merging #4460 (3c2d65b) into master (3645eb1) will decrease coverage by 2%.
The diff coverage is 89%.

@@           Coverage Diff           @@
##           master   #4460    +/-   ##
=======================================
- Coverage      93%     91%    -2%     
=======================================
  Files         160     159     -1     
  Lines       11402   11365    -37     
=======================================
- Hits        10654   10358   -296     
- Misses        748    1007   +259     

[tchaton]

Great addition.

Would you mind adding some tests ?

Best,
T.C

[akihironitta]

I don't think I am capable of writing tests for collect_init_args and get_init_args. It would be wonderful if someone helps me...

[akihironitta]

I need help in adding the tests for:

• get_init_args
• collect_init_args

I couldn't trace what get_init_args behaves, so I'm wondering if someone who's familiar with this part could add the tests...

[ethanwharris]

@akihironitta I added tests for get_init_args and collect_init_args, hope that helps!

[akihironitta]

@ethanwharris Thank you so much for your help!

mind re-review