neithere / argh

An argparse wrapper that doesn't make you say "argh" each time you deal with it.
http://argh.rtfd.org
GNU Lesser General Public License v3.0
369 stars 55 forks source link

fix: issue #216 #217

Closed thorwhalen closed 8 months ago

thorwhalen commented 8 months ago

Replaced

 if args and (args[0] in cls.BASIC_TYPES):

with

 if args and (args[0] in cls.BASIC_TYPES):
neithere commented 8 months ago

Hi @thorwhalen, thank you for the report and the fix!

Could you please provide a minimal example to repro the bug? It would be great to update the unit tests along with the fix.

thorwhalen commented 8 months ago

Hi @thorwhalen, thank you for the report and the fix!

Could you please provide a minimal example to repro the bug? It would be great to update the unit tests along with the fix.

from typing import Optional, List

def func(x: Optional[List] = None):
        return 'hello'

if __name__ == '__main__':
    import argh

    argh.dispatch_command(func)
thorwhalen commented 8 months ago

Hi @thorwhalen, thank you for the report and the fix! Could you please provide a minimal example to repro the bug? It would be great to update the unit tests along with the fix.

from typing import Optional, List

def func(x: Optional[List] = None):
        return 'hello'

if __name__ == '__main__':
    import argh

    argh.dispatch_command(func)

Given this, perhaps my PR is not the right fix. Perhaps it's something to do with Optional[List] in particular (I tried Optional[Tuple] and it doesn't give me the same problem.

Note: I'm on python 3.10.13, on a mac.

neithere commented 8 months ago

Aha, thanks, got it! Optional[list] or list | None works but Optional[List] is broken. I'll take a closer look soon.

P.S.: if you don't need to support older Pythons, you can simply replace List with list.

neithere commented 8 months ago

Thank you Thor, the final fix was indeed a bit different (see 8f270d01c6) but your effort and your example have helped me pinpoint the exact problem and solve it there. Will ship the fix in 0.31.1 today.

thorwhalen commented 7 months ago

Great. Thanks. I'll update to 0.31.1 now.