anntzer / defopt

Effortless argument parser
https://pypi.org/project/defopt/
MIT License
214 stars 11 forks source link

Reject duplicate options for a single parameter rather than silently dropping one #110

Closed lordmauve closed 1 year ago

lordmauve commented 2 years ago

When given the same argument multiple times, defopt silently discards all but the last:

>>> import defopt
>>> def foo(*, directory: str):
...     print(directory)
>>> with suppress(BaseException): defopt.run(foo, argv=['--directory', 'abc'])
abc
>>> with suppress(BaseException): defopt.run(foo, argv=['--directory', 'abc', '--directory', 'def'])
def

I find this surprising, and a user of one of my tools was surprised that it was accepting two directories but silently only acting on the last one. He was expecting it to act on both.

I note that argparse too silently discards repeated options:

>>> import argparse
>>> p = argparse.ArgumentParser()
>>> p.add_argument('--directory');
>>> p.parse_args(['--directory', 'abc', '--directory', 'def'])
Namespace(directory='def')

This behaviour comes from argparse._StoreAction, where it doesn't check that a value has previously been assigned to the namespace. But one could overwrite it by defining a new Action that does check.

A colleague noted that this overriding behaviour exists in other Unix tools and perhaps for good reason:

$ alias ls="ls --color=always"
$ ls --color=never
... prints without color 

But that's inconsistent with the view that we're just calling a Python function. In Python, repeating a keyword argument throws a TypeError or a SyntaxError:

>>> foo(directory='abc', directory='def')
  File "<stdin>", line 1
SyntaxError: keyword argument repeated: directory
>>> foo(directory='abc', **dict(directory='def'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __main__.foo() got multiple values for keyword argument 'directory'

__main__.foo() got multiple values for keyword argument 'directory'
anntzer commented 2 years ago

I think this would be better served by #95? The default behavior of later args overwriting earlier ones is very standard for command line tools and I would be loath to even provide a knob to make that error out (well, unless you can convince the argparse maintainers that this would be good to have -- if this indeed goes into e.g. Py3.12 I could backport whatever changes they make to argparse), especially as that doesn't really solve your original issue whereas #95 does.

OTOH someone actually needs to write the support for Annotated suggested in #95 :-)

anntzer commented 1 year ago

I will close this as one part is covered by #95 and the other should go to the stdlib. Feel free to reply if you would like to argue otherwise.