[WIP] Argparser Improvements

[nateraw]

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 to update the docs?
• Did you write any new necessary tests?

What does this PR do?

Fixes # (issue).

PR review

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 🙃

[codecov]

Codecov Report

Merging #293 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #293      +/-   ##
==========================================
- Coverage   81.91%   81.88%   -0.03%     
==========================================
  Files          98       98              
  Lines        5495     5481      -14     
==========================================
- Hits         4501     4488      -13     
+ Misses        994      993       -1     

Flag	Coverage Δ
#cpu	23.51% <50.00%> (+0.02%)	⬆️
#pytest	23.51% <50.00%> (+0.02%)	⬆️
#unittests	81.31% <100.00%> (-0.03%)	⬇️

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

Impacted Files	Coverage Δ
pl_bolts/utils/arguments.py	96.38% <100.00%> (+0.23%)	⬆️
pl_bolts/datasets/ssl_amdim_datasets.py	73.23% <0.00%> (-1.09%)	⬇️
pl_bolts/optimizers/lars_scheduling.py	95.23% <0.00%> (-0.51%)	⬇️
..._bolts/models/self_supervised/moco/moco2_module.py	78.74% <0.00%> (-0.50%)	⬇️
pl_bolts/datasets/imagenet_dataset.py	18.78% <0.00%> (-0.38%)	⬇️
pl_bolts/models/rl/common/gym_wrappers.py	89.65% <0.00%> (-0.27%)	⬇️
...ts/models/autoencoders/basic_ae/basic_ae_module.py	87.83% <0.00%> (-0.17%)	⬇️
pl_bolts/losses/self_supervised_learning.py	77.56% <0.00%> (-0.15%)	⬇️
.../models/autoencoders/basic_vae/basic_vae_module.py	91.48% <0.00%> (-0.09%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f3f580...25ad03b. Read the comment docs.

[pep8speaks]

Hello @nateraw! Thanks for updating this PR.

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-10-21 21:48:21 UTC

[edenlightning]

@awaelchli

[teddykoker]

Add docs please :)

[teddykoker]

Honestly I would prefer:

parser.add("model", MyLightningModule)
parser.add("data", MyDataModule)

That way the use could always add any arbitrary object they want

[nateraw]

Honestly I would prefer:

parser.add("model", MyLightningModule)
parser.add("data", MyDataModule)

That way the use could always add any arbitrary object they want

+1 to this. that's what add_object_args was before. which do you think is better function name? .add or .add_object_args? I figured the latter sounds more like argparse, but idc.

[odedbd]

@nateraw are the arguments for the different objects added into ArgumentParser groups (using add_argument_group)? I've read the code and couldn't find add_argument_group being called.

I think it would significantly help the clarity of the command line help.

Also, regarding your question to @teddykoker I prefer using add_object_args, or perhaps even add_object_specific_args, which conforms to the norm suggested in the documentation for add_model_specific_args.

[Borda]

@nateraw is it still WIP, how is it going?

[Borda]

seems dead, right? @nateraw feel free to reopen if suitable...