Behaviour of fire for variadic arguments

[pranavgupta1234]

I have some codebase which uses argparse and wanted to migrate into python-fire. Now there I have option of specifying nargs as different flags to get required arguments. Now say I have one flag which uses '+' and one uses '*' to get arguments. How can I wrap all this stuff in a function so that fire converts into desired cli automatically. Actually I want to get variable size list for different flags.
Consider this testfire.py

def special(*args, **kwargs):
  print(args)
  print(kwargs)

python testfire.py special --par1 haha --par2 a b c
produces:

('b', 'c')
{'par1': 'haha', 'par2': 'a'}

I would like to achieve is that to be able to have these name of par1 and par2 integrated and for par2 I want to get a, b, c in list.
Thanks

[dbieber]

python testfire.py special --par1 haha --par2 [a,b,c] is one way.

[pranavgupta1234]

Thanks for the quick reply @dbieber. par1 and par2 will still not be visible in fire help area, I want user to know about these par1 and par2 flags. Moreover any other workaround ?

[dbieber]

For a flag to appear in the help text, it has to appear as an argument in the function:

For example:

def special(par1, par2):
  print(par1)
  print(par2)

You could also mention par1 and par2 in the docstring, which will be shown with the help text.

[pranavgupta1234]

Actually I have two flags in my original code, one with nargs as '+' and other as '*' and now I want to create my function in best possible way to cover these aspects. What do you think can be the best way ?
Is there any other way to combine a, b and c as a list and get tied to a specific argument, because the way you mentioned above is shell dependent.

[dbieber]

If you use quotes, it won't be shell dependent.
python testfire.py special --par1 haha --par2 "[a,b,c]".

If you share your original code, I can better tell you how to write it with Fire. Please share not just the argparse code, but also the lines where the arguments get used, as this is the most important part for informing how to translate your CLI to Fire.

[pranavgupta1234]

Actually I am working on deepmind's kapitan https://github.com/deepmind/kapitan, and the main cli file containing logic for argparse is https://github.com/deepmind/kapitan/blob/master/kapitan/cli.py
Issue #kapicorp/kapitan#240

[dbieber]

I hope the following is helpful - here's what I put together for translating cli.py to use Fire:

---

So, currently it looks like this:

    if cmd == 'eval':
    ...

    elif cmd == 'compile':
    ...

    elif cmd == 'inventory':
    ...

    elif cmd == 'searchvar':
    ...

    elif cmd == 'lint':
    ...

    elif cmd == 'init':
    ...

    elif cmd == 'secrets':
    ...

As a first step, I recommend moving that to a structure more like:

def eval(jsonnet_file, search_paths, vars, output):
    ...
def compile(search_paths, ignore_version_check, secrets_path, inventory_path, output_path, parallelism, targets, ref_controller, prune, indent, reveal, cache, cache_paths):
    ...
def inventory(pattern, target_name, inventory_path, flat)
    ...
def searchvar(searchvar, inventory_path, pretty_print):
    ...
def lint(fail_on_warning, skip_class_checks, skip_yamllint, inventory_path, search_secrets, secrets_path, compiled_path):
    ...
def initialise_skeleton(directory):
    ...
def secrets(secrets_path, write, reveal, update, validate_targets, ...):
    # This one needs a bit more work since some of the args are extracted later.
    ...

And you don't even need to define lint, because it's already available as start_lint. You can similarly not define searchvar and initialize_skeleton since they're already defined.

So you just need to define write functions for eval, compile, inventory, and secrets.

I'll do eval here as an example.

Here's the current implementation:

  if cmd == 'eval':
        file_path = args.jsonnet_file
        search_paths = [os.path.abspath(path) for path in args.search_paths]
        ext_vars = {}
        if args.vars:
            ext_vars = dict(var.split('=') for var in args.vars)
        json_output = None

        def _search_imports(cwd, imp):
            return search_imports(cwd, imp, search_paths)

        json_output = jsonnet_file(file_path, import_callback=_search_imports,
                                   native_callbacks=resource_callbacks(search_paths),
                                   ext_vars=ext_vars)
        if args.output == 'yaml':
            json_obj = json.loads(json_output)
            yaml.safe_dump(json_obj, sys.stdout, default_flow_style=False)
        elif json_output:
            print(json_output)

Here's how you would modify it for fire:

def eval(jsonnet_file, search_paths, vars, output):
        file_path = jsonnet_file
        search_paths = [os.path.abspath(path) for path in search_paths]
        ext_vars = {}
        if vars:
            ext_vars = dict(var.split('=') for var in vars)
        json_output = None

        def _search_imports(cwd, imp):
            return search_imports(cwd, imp, search_paths)

        json_output = jsonnet_file(file_path, import_callback=_search_imports,
                                   native_callbacks=resource_callbacks(search_paths),
                                   ext_vars=ext_vars)
        if output == 'yaml':
            json_obj = json.loads(json_output)
            yaml.safe_dump(json_obj, sys.stdout, default_flow_style=False)
        elif json_output:
            print(json_output)

You could also modify the signature to be:
def eval(jsonnet_file, output, *search_paths, **vars):

In that case, you can even remove the splitting logic: ext_vars = dict(var.split('=') for var in vars) since vars will already be parsed.

Once you have eval, compile, inventory, and secrets defined, you can do something like this:

fire.Fire({'eval': eval, 'compile': compile, 'inventory': inventory, 'searchvar': searchvar, 'lint': start_lint, 'init': initialise_skeleton, 'secrets': secrets})

Or if you want greater flexibility (but a messier CLI since it will pick up extra objects beyond the ones you want to expose), you can simply do fire.Fire() with no args.

---

If you use def eval(jsonnet_file, search_paths, vars, output): as the signature for eval then usage is:

python cli.py jsonfile yaml --search-paths="[path1,path2]" --vars="['var1=val1','var2=val2']" (This is obviously messy and error prone; if you take this route I recommend making vars just a string, rather than a list, and parsing it yourself. You're already doing some of the parsing yourself anyway with the line "ext_vars = dict(var.split('=') for var in vars)").

If you use def eval(jsonnet_file, output, *search_paths, **vars): as the signature for eval then usage is:

python cli.py jsonfile yaml path1 path2 --var1=val1 --var2=val2.

I think this last line is what you were asking about. It sounds like you want to combine the approaches, so can specify --search-paths path1 path2 rather than just path1 path2 but that isn't supported for *args. And relatedly you can't have multiple *args in a Python function, as you know.

[dbieber]

My recommendation is to just accept a string (using signature def eval(jsonnet_file, search_paths, vars, output):) and split the string on commas (both for search_paths and for vars).

[pranavgupta1234]

Thanks a lot @dbieber for such elaborate analysis. Much appreciated. I would discuss there as well and then finally migrate it to python-fire. I also think your recommended way is much cleaner.

[pranavgupta1234]

While using def eval(jsonnet_file, search_paths, vars, output) and feeding using --search_paths=a,b,c it parses them as tuple and returns (a,b,c). How to get it as string ?
I was wondering the reason why it parsed it as tuple. I mean --search-paths=\"a,b,c\" is not that cleaner to write every time.

[dbieber]

You can use decorators.SetParseFn to force it to be a string, but 1) that API isn't guaranteed stable in future versions, and 2) don't you want it as a tuple anyway?

In Fire, you generally don't require or declare a specific type for a specific arg. Instead, the value provided by the user determines the type. So, if users are likely to pass in both strings and tuples, then your code should be written to handle both strings and tuples. @fire.decorators.SetParseFn(search_paths=str) let's you force parsing as a string, but is less standard for fire usage.

[pranavgupta1234]

When I passed --search-paths=.,./temp it was read as string. I mean the behavior is not uniform.
Am I missing something ?

[dbieber]

The parsing logic is here https://github.com/google/python-fire/blob/master/fire/parser.py#L57.

It converts bare words to strings and then uses Python's literal eval. This is to give you the flexibility of using any of Python's primitive types without requiring you to use quotes for strings. If the arg doesn't parse in this way, it is kept as a string.

Determining types directly from the values is one of Fire's strengths but can also be a pain point when it gives unexpected behavior, like you're seeing now.

[pranavgupta1234]

Thanks @dbieber. I have now wrapped parsing stuff inside a function, just a bit more help about displaying basic project summary. I mean we have PROJECT_NAME, DESCRIPTION, VERSION etc, so are they also needed to be wrapped inside a function and then fired or is there some other way as well ?

[dbieber]

I'm not sure I understand what you're asking. Are PROJECT_NAME, DESCRIPTION, VERSION values to be set by the user, or are they part of kapitan?

[pranavgupta1234]

They are part of kapitan. Just to be displayed. Project name is kapitan, and project description is kapitan defined in one line.
While constructing the ArgumentParser object in argparse we use it like

parser = argparse.ArgumentParser(prog=PROJECT_NAME,
                                     description=DESCRIPTION)
parser.add_argument('--version', action='version', version=VERSION)

These are in fact displayed when we just type kapitan. How we can create an equivalent behavior in Fire ? Moreover after constructing all functions in fire is there some way to show the docstring of each function on kapitan -- --help.

[dbieber]

fire.Fire(your_component, name=PROJECT_NAME)

For description and version, you can wrap your functions in a class rather than a dict

class KapitanCLI(object):
  """Description here"""

  def __init__(self, version=False):
    if version:
      return VERSION

  def eval(self, jsonnet_file, search_paths, vars, output):
      ...
  def compile(self, search_paths, ignore_version_check, secrets_path, inventory_path, output_path, parallelism, targets, ref_controller, prune, indent, reveal, cache, cache_paths):
      ...
  def inventory(self, pattern, target_name, inventory_path, flat)
      ...
  def searchvar(self, searchvar, inventory_path, pretty_print):
      ...
  def lint(self, fail_on_warning, skip_class_checks, skip_yamllint, inventory_path, search_secrets, secrets_path, compiled_path):
      ...
  def initialise_skeleton(self, directory):
      ...
  def secrets(self, secrets_path, write, reveal, update, validate_targets, ...):
      # This one needs a bit more work since some of the args are extracted later.
      ...

if __name__ == '__main__':
  fire.Fire(KapitanCLI, name=PROJECT_NAME)

And for showing all the docstrings on kapitan --help, that is included in the next release (see helptext.py). The current version only shows the function docstrings when you request help for a particular command, not when you request help for the full CLI.

[pranavgupta1234]

Hey @dbieber , as mentioned in #108 , shorthand notation is indeed added in fire. But when I try to use

import fire

class BrokenCalculator(object):

  def __init__(self, offset=1):
      self._offset = offset

  def add(self, val, par):
    return val + par + self._offset

  def multiply(self, x, y):
    return x * y + self._offset

if __name__ == '__main__':
  fire.Fire(BrokenCalculator)

as $ python testfire.py add -v 2 -p 4
it doesn't work and throws an error TypeError: can only concatenate str (not "int") to str

Is the shorthand ability added for all types or only boolean ?

According to logic defined here

python-fire/fire/core.py

Line 840 in d15fa28

elif len(key) == 1:

it should work.

[dbieber]

Yes, it will work in the next release. That improvement wasn't available yet in 0.0.3.

[pranavgupta1234]

When are you planning for next release ?

[dbieber]

I'm not able to make a reliable time estimate atm - but the main thing we want to finish before the release is switching from helputils to helptext, the latter of which is still a work in progress.

[dbieber]

The release should be going out later today (July 26, 2019).