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

Support for boolean options as flags (instead of string True/False values) #546

Open
aguschin opened this issue Dec 27, 2022 · 1 comment · May be fixed by #600
Open

Support for boolean options as flags (instead of string True/False values) #546

aguschin opened this issue Dec 27, 2022 · 1 comment · May be fixed by #600
Labels
bug Something isn't working build Exporting models (building model packages) cli MLEM command-line interface p3-low Low priority

Comments

@aguschin
Copy link
Contributor

aguschin commented Dec 27, 2022 •
edited by mike0sv
Loading

Error with --current_env in mlem build venv
The docs indicate the option is boolean:
--current_env: Whether to install in the current virtual env, must be active [default: False]
but:

$ mlem build venv --model lyrics2emoji --current_env
╭─ Error ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ Option '--current_env' requires an argument.                                                                                                         │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
$ mlem build venv --model lyrics2emoji --current_env True
⏳️ Loading model from lyrics2emoji.mlem
💼 Detected the virtual env /Users/aguschin/Git/iterative/mlem/emoji/.venv
💼 Installing the required packages...
......
@aguschin aguschin added bug Something isn't working cli MLEM command-line interface build Exporting models (building model packages) p1-high High priority labels Dec 27, 2022
@mike0sv mike0sv changed the title Error with --current_env in mlem build venv Support for boolean options as flags (instead of string True/False values) Jan 19, 2023
@mike0sv
Copy link
Contributor

mike0sv commented Jan 19, 2023

For this to work we need to check that field that is the source of cli option has boolean type and add is_flag when we generate the option somewhere here

Need to be careful though because we also might need to alter option name. Eg if is_on has default value False, it makes sense that --is_on will make it True. But if is_on default value is True, we should have something like --not_is_on as an option name

@ykasimov ykasimov linked a pull request Jan 27, 2023 that will close this issue
Open
@mike0sv mike0sv mentioned this issue Feb 13, 2023
Closed
@omesser omesser added p3-low Low priority and removed p1-high High priority labels Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working build Exporting models (building model packages) cli MLEM command-line interface p3-low Low priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

546 boolean options flags ykasimov/mlem
3 participants
@mike0sv @aguschin @omesser