Automatic short flags #23

anntzer · 2017-03-01T20:09:20Z

See #21. Note that this PR depends on #20 (the first few commits are from that PR).

Turning off automatic generation of short flags can be achieved by replacing the short = kwargs.pop('short', None) line by short = kwargs.pop('short', {}) (and updating the docs). But I'd rather keep it on by default.

anntzer · 2017-03-04T15:23:39Z

Rebased.

anntzer · 2017-03-16T01:15:57Z

Kindly bumping the issue.

evanunderscore · 2017-03-16T04:47:08Z

Thanks for your work on this. I've been intending to get to it but haven't found a large enough block of time to read through it properly. I'll try to review it this weekend.

anntzer · 2017-03-16T05:06:14Z

No worries. I have been using https://github.com/neithere/argh for most of my projects until recently. I learnt to get around the kinks, but the main problem is that the project is essentially abandoned at that point. I am looking towards switching to defopt, as I think parsing the docstring using a widespread standard (as opposed to, say, docopt) and using standard annotations (rather than attaching ad hoc semantics to them) were very, very good ideas; however, there are some kinks that need to be ironed out before the switch is worth it.

evanunderscore · 2017-03-19T08:42:19Z

defopt.py

@@ -54,16 +54,18 @@ def run(*funcs, **kwargs):
    :param Callable funcs: Function or functions to process and run
    :param parsers: Dictionary mapping types to parsers to use for parsing
        function arguments.
-    :type parsers: Dict[type, Callable[[str], type]]
+    :type parsers: Optional[Dict[type, Callable[[str], type]]]


Is it necessary to mark this as optional? The documentation for it says "An optional argument with a default needn’t use the Optional qualifier on its type annotation". Is there anything about this argument in particular that distinguishes it from short or argv?

seems reasonable, changed

evanunderscore · 2017-03-19T09:21:03Z

defopt.py

+        count_initials = Counter(name[0] for name in sig.parameters
+                                 if name not in positionals)
+        short = {name.replace('_', '-'): name[0] for name in sig.parameters
+                 if name not in positionals and count_initials[name[0]] == 1}


I would prefer to use single letter variable names in list and dictionary comprehensions. This code works fine currently, but the difference in behavior between Python 2 and 3 has caused me some grief in the past. (The variables remain in the current scope in 2, but not in 3).

Switched to using generator expressions, which do not leak variables even on py2.

evanunderscore · 2017-03-19T09:24:10Z

examples/short.py

+You can add alternative short flags to arguments by passing a dictionary
+to `defopt.run` which maps flag names to single letters.  You can
+prevent automatic addition of short flags by passing an empty dictionary
+as argument.


These module docstrings are intended to be brief descriptions of what's happening in the example rather than full explanations of the features. Most of this probably belongs under the flags section in features.rst. (I have ended up duplicating a lot of documentation myself, which is not ideal.)

moved to features.rst

evanunderscore · 2017-03-19T09:34:30Z

I think there are some edge cases where the global short dict you pass in won't be able to replicate exactly what the auto short generation does once more than one command is involved, but I think we can solve that later on if it becomes a problem. There would have to be a more granular way to specify flag names. Perhaps we'll need a decorator-based approach as was proposed in #2 after all.

anntzer · 2017-03-19T19:02:45Z

What are the edge cases you're thinking about?

evanunderscore · 2017-03-20T07:52:03Z

docs/features.rst

+You can add alternative short flags to arguments by passing a dictionary
+to `defopt.run` which maps flag names to single letters.  You can
+prevent automatic addition of short flags by passing an empty dictionary
+as argument::


Should this be "as an argument"?

After rereading this section, I think it may need to be reworded - specifically "add alternative short flags" may be taken to imply that the given flags are used in addition to the generated ones, as opposed to overriding them entirely. How about something like this?

You can override these generated short flags by passing a dictionary to defopt.run which maps flag names to single letters::

[example]

Passing an empty dictionary will suppress automatic short flag generation without adding new flags.

I think the second part belongs after the example is it's not actually demonstrated in there. How does that seem to you?

Sounds good, fixed (with minor rewording).

evanunderscore · 2017-03-20T08:02:19Z

Consider:

def func1(bar: int = 1): pass
def func2(baz: int = 1): pass
def func3(bar: int = 1): pass
defopt.run(func1, func2, func3)

Each of these have a -b flag mapping to either bar or baz. If you then want to add baz to func3 (ie change the definition to def func3(bar: int = 1, baz: int = 1)), you lose the -b flag, and you can't restore it. Specifying {'bar': 'b', 'baz': 'b'} breaks for func3, and just {'bar': 'b'} doesn't fix func2 (though this could possibly work if the dict added to the defaults instead of overriding them entirely).

What are your thoughts on this?

anntzer · 2017-03-20T15:48:39Z

Perhaps short={"funcname:argname": "short", ...} when disambiguation is needed?

evanunderscore · 2017-03-22T10:23:56Z

That would work, but I'm not sure I like encoding information into the strings. Either way, I think we can defer this problem until the need arises. Once the documentation is cleared up I'm happy to merge this.

evanunderscore · 2017-03-23T09:43:35Z

Thanks again for your work. Merging.

evanunderscore reviewed Mar 19, 2017

View reviewed changes

evanunderscore reviewed Mar 20, 2017

View reviewed changes

Automatic short flags.

abe0959

anntzer mentioned this pull request Mar 22, 2017

Summarize subcommands #26

Merged

evanunderscore merged commit 6ac0c38 into anntzer:master Mar 23, 2017

anntzer deleted the autoshort branch June 19, 2017 03:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatic short flags #23

Automatic short flags #23

anntzer commented Mar 1, 2017

anntzer commented Mar 4, 2017

anntzer commented Mar 16, 2017

evanunderscore commented Mar 16, 2017

anntzer commented Mar 16, 2017

evanunderscore Mar 19, 2017

anntzer Mar 19, 2017

evanunderscore Mar 19, 2017

anntzer Mar 19, 2017

evanunderscore Mar 19, 2017

anntzer Mar 19, 2017

evanunderscore commented Mar 19, 2017

anntzer commented Mar 19, 2017

evanunderscore Mar 20, 2017

anntzer Mar 20, 2017

evanunderscore Mar 22, 2017

anntzer Mar 22, 2017

evanunderscore commented Mar 20, 2017

anntzer commented Mar 20, 2017

evanunderscore commented Mar 22, 2017

evanunderscore commented Mar 23, 2017

Automatic short flags #23

Automatic short flags #23

Conversation

anntzer commented Mar 1, 2017

anntzer commented Mar 4, 2017

anntzer commented Mar 16, 2017

evanunderscore commented Mar 16, 2017

anntzer commented Mar 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanunderscore commented Mar 19, 2017

anntzer commented Mar 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evanunderscore commented Mar 20, 2017

anntzer commented Mar 20, 2017

evanunderscore commented Mar 22, 2017

evanunderscore commented Mar 23, 2017