openage --forward -> opencage-api forward

opencage/batch.py

@@ -62,12 +62,12 @@ async def read_one_line(self, row):
         if self.options.input_columns:
             # input_columns option uses 1-based indexing
             address = [row[i-1] for i in self.options.input_columns]
-        elif self.options.reverse:
+        elif self.options.command == 'reverse':
             address = [row[1], row[2]]
         else:
             address = [row[1]]
 
-        if self.options.reverse and len(address) != 2:
+        if self.options.command == 'reverse' and len(address) != 2:
             self.log(f"Expected two comma-separated values for reverse geocoding, got {address}")
 
         return { 'id': row[0], 'address': ','.join(address) }
@@ -100,7 +100,7 @@ async def _geocode_one_address():
                 params = { 'no_annotations': 1, **self.options.extra_params }
 
                 try:
-                    if self.options.reverse:
+                    if self.options.command == 'reverse':
                         lon, lat = address.split(',')
                         geocoding_results = await geocoder.reverse_geocode_async(lon, lat, **params)
                     else:

opencage/command_line.py

@@ -21,28 +21,33 @@ def main(args=sys.argv[1:]):
 
 
 def parse_args(args):
-    parser = argparse.ArgumentParser(description="opencage")
-
-    parser.add_argument("--api-key", required=True, type=api_key_type, help="your OpenCage API key")
-    parser.add_argument("--input", required=True, type=argparse.FileType('r', encoding='utf-8'), help="input file name (one query per line)")
-    parser.add_argument("--output", required=True, type=argparse.FileType('x', encoding='utf-8'), help="output file name")
-
-    group = parser.add_mutually_exclusive_group(required=True)
-    group.add_argument("--forward", action="store_true", help="use forward geocoding")
-    group.add_argument("--reverse", action="store_true", help="use reverse geocoding")
-
-    parser.add_argument("--limit", type=int, default=0, help="number of lines to read from the input file")
-    parser.add_argument("--has-headers", action="store_true", help="if the first row should be treated as a header row")
-    parser.add_argument("--input-columns", type=comma_separated_type(int), default="", help="comma separated list of integers")
-    parser.add_argument("--add-columns", type=comma_separated_type(str), default="_type,country,county,city,postcode,road,house_number,confidence,formatted", help="comma separated list of output columns")
-    parser.add_argument("--workers", type=ranged_type(int, 1, 20), default=1, help="number of parallel workers")
-    parser.add_argument("--timeout", type=ranged_type(int, 1, 60), default=1, help="timeout in seconds")
-    parser.add_argument("--retries", type=ranged_type(int, 1, 60), default=10, help="number of retries")
-    parser.add_argument("--dry-run", action="store_true", help="only parse the input file")
-    parser.add_argument("--api-domain", type=str, default="api.opencagedata.com", help="API domain")
-    parser.add_argument("--extra-params", type=comma_separated_dict_type, default="", help="extra parameters appended to request URLs")
-    parser.add_argument("--no-progress", action="store_true", help="no progress bar")
-    parser.add_argument("--quiet", action="store_true", help="skip the 'all done' message")
+    parser = argparse.ArgumentParser(description="Opencage CLI")
+
+    subparsers = parser.add_subparsers(dest='command')
+    subparsers.required = True
+
+    subparser_forward = subparsers.add_parser('forward', help="Forward geocode a file (address to coordinates)")
+    subparser_reverse = subparsers.add_parser('reverse', help="Reverse geocode a file (coordinates to address)")
+
+    for sp in [subparser_forward, subparser_reverse]:
+
+        sp.add_argument("--api-key", required=True, type=api_key_type, help="your OpenCage API key")
+        sp.add_argument("--input", required=True, type=argparse.FileType('r', encoding='utf-8'), help="input file name (one query per line)")
+        sp.add_argument("--output", required=True, type=argparse.FileType('x', encoding='utf-8'), help="output file name")
+
+        sp.add_argument("--limit", type=int, default=0, help="number of lines to read from the input file")
+        sp.add_argument("--has-headers", action="store_true", help="if the first row should be treated as a header row")
+        sp.add_argument("--input-columns", type=comma_separated_type(int), default="", help="comma separated list of integers")
+        sp.add_argument("--add-columns", type=comma_separated_type(str), default="_type,country,county,city,postcode,road,house_number,confidence,formatted", help="comma separated list of output columns")
+        sp.add_argument("--workers", type=ranged_type(int, 1, 20), default=1, help="number of parallel workers")
+        sp.add_argument("--timeout", type=ranged_type(int, 1, 60), default=1, help="timeout in seconds")
+        sp.add_argument("--retries", type=ranged_type(int, 1, 60), default=10, help="number of retries")
+        sp.add_argument("--dry-run", action="store_true", help="only parse the input file")
+        sp.add_argument("--api-domain", type=str, default="api.opencagedata.com", help="API domain")
+        sp.add_argument("--extra-params", type=comma_separated_dict_type, default="", help="extra parameters appended to request URLs")
+        sp.add_argument("--no-progress", action="store_true", help="no progress bar")
+        sp.add_argument("--quiet", action="store_true", help="skip the 'all done' message")
+
 
     return parser.parse_args(args)
 

test/test_cli_args.py

@@ -21,13 +21,23 @@ def assert_parse_args_error(args, message, capfd):
 def test_required_arguments(capfd):
     assert_parse_args_error(
         [],
-        'the following arguments are required: --api-key, --input, --output',
+        'the following arguments are required: command',
+        capfd
+    )
+
+def test_invalid_command(capfd):
+    assert_parse_args_error(
+        [
+            "singasong"
+        ],
+        'argument command: invalid choice',
         capfd
     )
 
 def test_invalid_api_key(capfd):
     assert_parse_args_error(
         [
+            "forward",
             "--api-key", "invalid",
             "--input", "test/fixtures/input.txt",
             "--output", "test/fixtures/output.csv",
@@ -40,6 +50,7 @@ def test_invalid_api_key(capfd):
 def test_existing_output_file(capfd):
     assert_parse_args_error(
         [
+            "forward",
             "--api-key", "oc_gc_12345678901234567890123456789012",
             "--input", "test/fixtures/input.txt",
             "--output", "test/fixtures/input.txt",
@@ -49,20 +60,10 @@ def test_existing_output_file(capfd):
         capfd
     )
 
-def test_requires_forward_or_reverse(capfd):
-    assert_parse_args_error(
-        [
-            "--api-key", "oc_gc_12345678901234567890123456789012",
-            "--input", "test/fixtures/input.txt",
-            "--output", "test/fixtures/output.csv",
-        ],
-        'one of the arguments --forward --reverse is required',
-        capfd
-    )
-
 def test_argument_range(capfd):
     assert_parse_args_error(
         [
+            "forward",
             "--api-key", "oc_gc_12345678901234567890123456789012",
             "--input", "test/fixtures/input.txt",
             "--output", "test/fixtures/output.csv",
@@ -75,10 +76,10 @@ def test_argument_range(capfd):
 
 def test_full_argument_list():
     args = parse_args([
+        "reverse",
         "--api-key", "oc_gc_12345678901234567890123456789012",
         "--input", "test/fixtures/input.txt",
         "--output", "test/fixtures/output.csv",
-        "--reverse",
         "--has-header",
         "--input-columns", "1,2",
         "--add-columns", "city,postcode",
@@ -93,11 +94,10 @@ def test_full_argument_list():
         "--quiet"
     ])
 
+    assert args.command == "reverse"
     assert args.api_key == "oc_gc_12345678901234567890123456789012"
     assert args.input.name == "test/fixtures/input.txt"
     assert args.output.name == "test/fixtures/output.csv"
-    assert args.reverse is True
-    assert args.forward is False
     assert args.has_headers is True
     assert args.input_columns == [1, 2]
     assert args.add_columns == ["city", "postcode"]
@@ -113,14 +113,13 @@ def test_full_argument_list():
 
 def test_defaults():
     args = parse_args([
+        "forward",
         "--api-key", "12345678901234567890123456789012",
         "--input", "test/fixtures/input.txt",
-        "--output", "test/fixtures/output.csv",
-        "--reverse"
+        "--output", "test/fixtures/output.csv"
     ])
 
-    assert args.reverse is True
-    assert args.forward is False
+    assert args.command == "forward"
     assert args.limit == 0
     assert args.has_headers is False
     assert args.input_columns == []

test/test_cli_run.py

@@ -15,7 +15,7 @@ def around():
 def assert_output(path, length, lines):
     assert pathlib.Path(path).exists()
 
-    with open(path, "r") as f:
+    with open(path, "r", encoding="utf-8") as f:
         actual = f.readlines()
         assert len(actual) == length
 
@@ -24,10 +24,10 @@ def assert_output(path, length, lines):
 
 def test_forward():
     main([
+        "forward",
         "--api-key", "6d0e711d72d74daeb2b0bfd2a5cdfdba",
         "--input", "test/fixtures/forward.csv",
         "--output", "test/fixtures/output.csv",
-        "--forward",
         "--input-columns", "2,3,4",
     ])
 
@@ -39,10 +39,10 @@ def test_forward():
 
 def test_reverse():
     main([
+        "reverse",
         "--api-key", "6d0e711d72d74daeb2b0bfd2a5cdfdba",
         "--input", "test/fixtures/reverse.csv",
         "--output", "test/fixtures/output.csv",
-        "--reverse",
         "--add-columns", "city,postcode"
     ])