LightningCLI natively support callback list append

[mauvilsa]

What does this PR do?

Uses a new version of jsonargparse which implements the support for append to list. This allows to remove the placeholder LightningArgumentParser._convert_argv_issue_85. Documentation of this new jsonargparse feature can be seen at https://jsonargparse.readthedocs.io/en/latest/#list-append.

Does your PR introduce any breaking changes? If yes, please list them.

It does not introduce breaking changes in the programming API. However, it does introduce a change in the command line syntax. When there is a type hint that uses a List it is possible to either replace the previous argument value or append to the list. The append uses a new syntax which is suffixing the key with +. For example to add a callback from the command line would be either --trainer.callbacks+=CallbackName or --trainer.callbacks+ CallbackName. When a previous list value is replaced instead of appended to, a warning is printed.

Before submitting

• Was this discussed/approved via a GitHub issue? (not for typos and docs)
• 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?
• Did you make sure to update the documentation with your changes? (if necessary)
• Did you write any new necessary tests? (not for typos and docs)
• Did you verify new and existing tests pass locally with your changes?
• Did you list all the breaking changes introduced by this pull request?
• Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

• Is this pull request ready for review? (if not, please submit in draft mode)
• Check that all items from Before submitting are resolved
• Make sure the title is self-explanatory and the description concisely explains the PR
• Add labels and milestones (and optionally projects) to the PR so it can be classified

Did you have fun?

Make sure you had fun coding 🙃

[carmocca]

Super!

[akihironitta]

This new feature is really nice! 🚀

---

I'm testing out this feature with an example here: https://github.com/akihironitta/gist/blob/repro/13129-cli-callback-append/pl_cli_boring_model/main.py

$ python main.py --trainer.callbacks+=DeviceStatsMonitor

but I get the following error:

Traceback (most recent call last):
  File "/Users/nitta/work/github.com/akihironitta/gist/pl_cli_boring_model/main.py", line 70, in <module>
    main()
  File "/Users/nitta/work/github.com/akihironitta/gist/pl_cli_boring_model/main.py", line 58, in main
    cli = LightningCLI(
  File "/Users/nitta/work/github.com/PyTorchLightning/pytorch-lightning/pytorch_lightning/utilities/cli.py", line 418, in __init__
    self.instantiate_classes()
  File "/Users/nitta/work/github.com/PyTorchLightning/pytorch-lightning/pytorch_lightning/utilities/cli.py", line 558, in instantiate_classes
    self.trainer = self.instantiate_trainer()
  File "/Users/nitta/work/github.com/PyTorchLightning/pytorch-lightning/pytorch_lightning/utilities/cli.py", line 568, in instantiate_trainer
    return self._instantiate_trainer(trainer_config, extra_callbacks)
  File "/Users/nitta/work/github.com/PyTorchLightning/pytorch-lightning/pytorch_lightning/utilities/cli.py", line 572, in _instantiate_trainer
    config["callbacks"].extend(callbacks)
AttributeError: 'DeviceStatsMonitor' object has no attribute 'extend'

and confirmed that updating the CLI call to:

$ python main.py --trainer.callbacks+="[DeviceStatsMonitor]"

resolves the error. I might be missing something, but is this behaviour expected? Do users need to wrap the callback name with [...] in their command line?

---

@carmocca I think we should label this PR as "breaking change" because the following CLI usage will behave differently between 1.6.x and 1.7:

$ python ... \
    --trainer.callbacks=Callback1 \
    --trainer.callbacks=Callback2

[mauvilsa]

I'm testing out this feature with an example here: https://github.com/akihironitta/gist/blob/repro/13129-cli-callback-append/pl_cli_boring_model/main.py

$ python main.py --trainer.callbacks+=DeviceStatsMonitor

but I get the following error:

This is not expected. There shouldn't be a need to wrap the callback name with [...]. I will fix it.

[mauvilsa]

I'm testing out this feature with an example here: https://github.com/akihironitta/gist/blob/repro/13129-cli-callback-append/pl_cli_boring_model/main.py

$ python main.py --trainer.callbacks+=DeviceStatsMonitor

but I get the following error:

This is not expected. There shouldn't be a need to wrap the callback name with [...]. I will fix it.

It should be fixed now. The issue was that the type for callbacks is Union[List[Callback], Callback], meaning that it is valid to have a callback not in a list. With the latest change --trainer.callbacks=DeviceStatsMonitor would not be a list, but then LightningCLI handles it correctly and --trainer.callbacks+=DeviceStatsMonitor would be a list with a single element.

[akihironitta]

Confirmed it works now! Thank you! LGTM 🚀