Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: export python script that includes args from a datastack #1472

Open
davemfish opened this issue Dec 12, 2023 · 2 comments · Fixed by #1476
Open

CLI: export python script that includes args from a datastack #1472

davemfish opened this issue Dec 12, 2023 · 2 comments · Fixed by #1476
Assignees
Labels
enhancement New feature or request in progress This issue is actively being worked on

Comments

@davemfish
Copy link
Contributor

invest export-py does not currently accept a datastack arg, though the function that builds the python script can take an args dict. So this should be a trivial change to add a command-line arg to the parser.

I'm wanting this feature as it is the quickest way I can think of to run a model on a given datastack and drop into a python debugger on exceptions.

@davemfish davemfish added the enhancement New feature or request label Dec 12, 2023
@davemfish davemfish self-assigned this Dec 12, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 12, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 12, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 12, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 12, 2023
@davemfish davemfish added the in progress This issue is actively being worked on label Dec 12, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 13, 2023
davemfish added a commit to davemfish/invest that referenced this issue Dec 13, 2023
@davemfish
Copy link
Contributor Author

Emily pointed out:

it's a little redundant to have to provide the model name as an argument, when it's also known from the datastack. I see why that's necessary using the export_py subparser though. Another option would be to make a separate command for converting a datastack to a script - but that would be extra work for a pretty minor issue.

Another option could be to use mutually exclusive args, so either the model name is passed, or a datastack JSON. https://docs.python.org/3/library/argparse.html#mutual-exclusion

Example
invest export-py --model carbon (creates a python script with args dict and all empty string arg values)
invest export-py --datastack path/to/carbon.json (creates python script with args dict and values from json)
invest export-py --model carbon --datastack path/to/carbon.json (argparse raises error)

One downside here is that the model name arg is no longer a simple positional arg like it is with other subcommands (run, getspec). So that could be surprising to users.

We could also probably implement our own mutually exclusive rules that would do something like,
Allowed:
invest export-py carbon
invest export-py -d path/to/carbon.json

Not allowed:
invest export-py carbon -d path/to/carbon.json

@davemfish
Copy link
Contributor Author

Oops, I think I referenced this issue by mistake in an unrelated PR that closed it

@davemfish davemfish reopened this Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in progress This issue is actively being worked on
Projects
None yet
1 participant