Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Flash Zero: Allow to customize LightningCLI #1437

Open
daavoo opened this issue Aug 31, 2022 · 4 comments
Open

Flash Zero: Allow to customize LightningCLI #1437

daavoo opened this issue Aug 31, 2022 · 4 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@daavoo
Copy link

daavoo commented Aug 31, 2022

馃殌 Feature

Expose options to customize the instantiation of LightningCLI in Flash Zero commands.

Motivation

I would like to be able to customize the instantiation. For example, to not use SaveConfigCallback which is hardcoded in:

https://github.com/Lightning-AI/lightning-flash/blob/0253d71588b582da326bb01ef116ebd7cd61f21e/flash/core/utilities/flash_cli.py#L166

Pitch

flash --save-config-callback False {task}

Alternatives

Do not use Flash Zero directly and instantiate LightningCLI manually.

@daavoo daavoo added enhancement New feature or request help wanted Extra attention is needed labels Aug 31, 2022
@krshrimali
Copy link
Contributor

Hi, @daavoo - Thank you for creating the issue! I think it makes sense, note that LightningCLI has save_config_callback and we can set it to None in LightningCLI but not in Flash Zero.

Myabe, we can think on the name, as having a boolean flag for save-config-callback might confuse users with save_config_callback in LightningCLI. Let's use, --save-config? cc: @ethanwharris - What do you think?

Just wondering, if you would like to take this up and create a PR? If you think you can help setup a draft PR, I can take it from there. :) Otherwise, I can help add this flag.

@daavoo
Copy link
Author

daavoo commented Sep 1, 2022

Hi, @daavoo - Thank you for creating the issue! I think it makes sense, note that LightningCLI has save_config_callback and we can set it to None in LightningCLI but not in Flash Zero.

Yes, in the example I set it to False but it has the same effect as None because the check is if self.save_config_callback.

Myabe, we can think on the name, as having a boolean flag for save-config-callback might confuse users with save_config_callback in LightningCLI. Let's use, --save-config? cc: @ethanwharris - What do you think?

No strong opinion on name.

Just wondering, if you would like to take this up and create a PR? If you think you can help setup a draft PR, I can take it from there. :) Otherwise, I can help add this flag.

Sure thing. I wanted to open the issue first to check if it made sense for you

@krshrimali
Copy link
Contributor

I think a PR to add a boolean flag --save-config to enable/disable save config callback will be a good idea! Regarding the name, if there are any other suggestions, we can rename it quickly in the PR before merging. :)

I'll be looking forward to your PR, thank you very much!

@Borda
Copy link
Member

Borda commented Dec 5, 2022

@daavoo are you interested in looking at it?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants