Open napulen opened 3 years ago
For the -export
, --export
would work.
If you have arguments for choosing the current convention, explain them and if they're compelling, the current convention can stay.
@napulen Should be resolved in the most recent commits to dev, let me know if this resolves the issue fully!
At a first glance
parser.add_argument(
"-t",
nargs="?",
required=True,
type=lambda fname: check_file_validity(fname, ["txt", "mei"]),
help="Flag indicating whether the inputs will be mei or txt files",
)
that could be
parser.add_argument(
"-t",
nargs="?",
required=True,
choices=["txt", "mei"],
help="Flag indicating whether the inputs will be mei or txt files",
)
Here is the relevant doc: https://docs.python.org/3/library/argparse.html#choices
Not sure about the required=True
. By definition, an option specified by a dash is an optional argument. Need to look at your logic further to assess both use cases (the lambda
and the required
).
Yeah I left the -t and set it as required just to have it stick with a flag as you had done in the initial issue comment, I can change it up though. In regards to the choices option, I will change it to that.. got caught up trying to reuse what I had built previously.
EDIT: In fact, upon second glance I can remove the whole use of the check_file_validity() function and simplify the driver further. Changes will be reflected in the next dev commit.
No problem. Looking better already. In the README
too :+1:
Re. the type, if you want to enforce a certain type by default without making it a required
arg, you can just set default="mei"
or similar. Analog to setting a default value on an input argument to a function def foo(requiredArg, optional="mei")
.
@napulen updated so that instead of providing a -t, the field is set default to mei, (so mei2vol -W filename.mei
or mei2vol -N filename.mei
) and if they wish to input multiple mei through a txt file it would be mei2vol -W txt filename.txt
.
EDIT: switched back to specifying mei/txt due to positional errors when passing multiple files in one go.. may come back to it later but the current changes are reflected in the README.
Also was able to remove check_file_validity as it was clearly no longer needed with in current iteration of the driver.
This one can create ambiguity. The way I read it, the code wouldn't be able to tell if mei2vol txt txt2
is two mei files named txt
txt
or one mei file of txt
type.
In general, the -t
with default="mei"
seems less obscure to me.
Not a big deal, but pointing it out
The general convention for optional args in a (unix-like) CLI is:
-o
single-letter switch for single-dashed options and--option-in-cli
dashed-separated words for double-dashed optionsThese usually go in pairs, the shorthand and the full spelled option name. For example:
The current notation for
-Wtxt
or-export
conflicts with this convention.For
-Wtxt
I would recommend to have a separate switch specifying thetype
of file. For example-W -t txt
; where-t
meanstype
and it can be a choice between["mei", "txt"]
or something similar.