google / python-fire

Python Fire is a library for automatically generating command line interfaces (CLIs) from absolutely any Python object.
Other
26.95k stars 1.44k forks source link

Optional type args are mistyped in help #508

Open impredicative opened 5 months ago

impredicative commented 5 months ago

Type annotations that use typing.Optional are incorrectly documented when showing help.

Example 1

from typing import Optional

import fire

def main(arg: Optional[str] = 'something'):
    """Do something with arg."""

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

The corresponding help output is:

INFO: Showing help with the command 'firetest.py -- --help'.

NAME
    firetest.py - Do something with arg.

SYNOPSIS
    firetest.py <flags>

DESCRIPTION
    Do something with arg.

FLAGS
    -a, --arg=ARG
        Type: Optional
        Default: 'something'

Here, Type: Optional is totally incorrect. It should be Optional[str]. My use of str is just an example and it can be anything.

Example 2

from typing import Optional

import fire

def main(arg: Optional[str] = None):
    """Do something with arg."""

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

The corresponding help output is:

INFO: Showing help with the command 'firetest.py -- --help'.

NAME
    firetest.py - Do something with arg.

SYNOPSIS
    firetest.py <flags>

DESCRIPTION
    Do something with arg.

FLAGS
    -a, --arg=ARG
        Type: Optional[Optional]
        Default: None

Here, Type: Optional[Optional] is even more incorrect. It should be Optional[str]. My use of str is just an example and it can be anything.

dbieber commented 5 months ago

Thanks for reporting this. Looks like a bug to me. Next steps are to add a test case and identify and resolve the issue.

Jemeljanov commented 4 months ago

Hi @dbieber,

I submitted pr #513 to solve this bug and added tests, please let me know your opinion.

Jemeljanov commented 4 months ago

Hi @dbieber,

Updated pr #513 to work with the typing package, waiting for your review on the last commit.

impredicative commented 2 months ago

Given that fire is seemingly dead and this bug really harms the displayed help, I figure it's wise to move away from fire. Any decent LLM, e.g. gpt-4o, can replace the use of fire in a module with a different package, e.g. click.