From 99a0f61b353ef02863af9cdaa136485546baacc5 Mon Sep 17 00:00:00 2001
From: Drew <drewrisinger@users.noreply.github.com>
Date: Wed, 9 Jan 2019 23:58:11 -0500
Subject: [PATCH] artiq_client: remove custom input validation for built-in
 argparse (#1185)

---
 artiq/frontend/artiq_client.py | 58 ++++++++++++++++------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff --git a/artiq/frontend/artiq_client.py b/artiq/frontend/artiq_client.py
index 7f5c63d7c..cc1b45858 100755
--- a/artiq/frontend/artiq_client.py
+++ b/artiq/frontend/artiq_client.py
@@ -1,4 +1,10 @@
 #!/usr/bin/env python3
+"""
+Client to send commands to :mod:`artiq_master` and display results locally.
+
+The client can perform actions such as accessing/setting datasets,
+scanning devices, scheduling experiments, and looking for experiments/devices.
+"""
 
 import argparse
 import logging
@@ -15,7 +21,7 @@ from artiq.protocols.pc_rpc import Client
 from artiq.protocols.sync_struct import Subscriber
 from artiq.protocols.broadcast import Receiver
 from artiq.protocols import pyon
-from artiq.tools import short_format
+from artiq.tools import short_format, parse_arguments
 
 
 def clear_screen():
@@ -46,7 +52,8 @@ def get_argparser():
                                  "scheduling, default: %(default)s)")
     parser_add.add_argument("-t", "--timed", default=None, type=str,
                             help="set a due date for the experiment")
-    parser_add.add_argument("-f", "--flush", default=False, action="store_true",
+    parser_add.add_argument("-f", "--flush", default=False,
+                            action="store_true",
                             help="flush the pipeline before preparing "
                             "the experiment")
     parser_add.add_argument("-R", "--repository", default=False,
@@ -80,10 +87,12 @@ def get_argparser():
                                     help="name of the dataset")
     parser_set_dataset.add_argument("value", metavar="VALUE",
                                     help="value in PYON format")
-    parser_set_dataset.add_argument("-p", "--persist", action="store_true",
-                                    help="make the dataset persistent")
-    parser_set_dataset.add_argument("-n", "--no-persist", action="store_true",
-                                    help="make the dataset non-persistent")
+
+    persist_group = parser_set_dataset.add_mutually_exclusive_group()
+    persist_group.add_argument("-p", "--persist", action="store_true",
+                               help="make the dataset persistent")
+    persist_group.add_argument("-n", "--no-persist", action="store_true",
+                               help="make the dataset non-persistent")
 
     parser_del_dataset = subparsers.add_parser(
         "del-dataset", help="delete a dataset")
@@ -93,7 +102,8 @@ def get_argparser():
         "show", help="show schedule, log, devices or datasets")
     parser_show.add_argument(
         "what", metavar="WHAT",
-        help="select object to show: schedule/log/devices/datasets")
+        choices=["schedule", "log", "ccb", "devices", "datasets"],
+        help="select object to show: %(choices)s")
 
     subparsers.add_parser(
         "scan-devices", help="trigger a device database (re)scan")
@@ -114,20 +124,11 @@ def get_argparser():
     return parser
 
 
-def _parse_arguments(arguments):
-    d = {}
-    for argument in arguments:
-        name, value = argument.split("=")
-        d[name] = pyon.decode(value)
-    return d
-
-
 def _action_submit(remote, args):
     try:
-        arguments = _parse_arguments(args.arguments)
-    except:
-        print("Failed to parse run arguments")
-        sys.exit(1)
+        arguments = parse_arguments(args.arguments)
+    except Exception as err:
+        raise ValueError("Failed to parse run arguments") from err
 
     expid = {
         "log_level": logging.WARNING + args.quiet*10 - args.verbose*10,
@@ -154,10 +155,6 @@ def _action_delete(remote, args):
 
 
 def _action_set_dataset(remote, args):
-    if args.persist and args.no_persist:
-        print("Options --persist and --no-persist cannot be specified "
-              "at the same time")
-        sys.exit(1)
     persist = None
     if args.persist:
         persist = True
@@ -190,13 +187,13 @@ def _action_ls(remote, args):
 def _show_schedule(schedule):
     clear_screen()
     if schedule:
-        l = sorted(schedule.items(),
-                   key=lambda x: (-x[1]["priority"],
-                                  x[1]["due_date"] or 0,
-                                  x[0]))
+        sorted_schedule = sorted(schedule.items(),
+                                 key=lambda x: (-x[1]["priority"],
+                                                x[1]["due_date"] or 0,
+                                                x[0]))
         table = PrettyTable(["RID", "Pipeline", "    Status    ", "Prio",
                              "Due date", "Revision", "File", "Class name"])
-        for rid, v in l:
+        for rid, v in sorted_schedule:
             row = [rid, v["pipeline"], v["status"], v["priority"]]
             if v["due_date"] is None:
                 row.append("")
@@ -252,6 +249,7 @@ def _run_subscriber(host, port, subscriber):
 
 def _show_dict(args, notifier_name, display_fun):
     d = dict()
+
     def init_d(x):
         d.clear()
         d.update(x)
@@ -299,8 +297,7 @@ def main():
         elif args.what == "datasets":
             _show_dict(args, "datasets", _show_datasets)
         else:
-            print("Unknown object to show, use -h to list valid names.")
-            sys.exit(1)
+            raise ValueError
     else:
         port = 3251 if args.port is None else args.port
         target_name = {
@@ -318,5 +315,6 @@ def main():
         finally:
             remote.close_rpc()
 
+
 if __name__ == "__main__":
     main()