Remove Gen 2-style --id argument.

doc/lsst.ap.verify/command-line-reference.rst

@@ -55,23 +55,17 @@ Required arguments are :option:`--dataset` and :option:`--output`.
       specific database system being used. If the database has been written to
       by a previous run, clear it by hand before running with ``--clean-run``.
 
-.. option:: -d, --data-query, --id <dataId>
+.. option:: -d, --data-query <dataId>
 
    **Butler data ID.**
 
    Specify data ID to process.
    This should use :ref:`dimension expression syntax <daf_butler_dimension_expressions>`, such as ``--data-query "visit=12345 and detector in (1..6) and band='g'"``.
 
    Multiple copies of this argument are allowed.
-   For compatibility with the syntax used by command line tasks, this flag with no argument processes all data IDs.
 
    If this argument is omitted, then all data IDs in the dataset will be processed.
 
-   .. warning::
-
-      The ``--id`` form of this argument is for consistency with Gen 2 command-line tasks, and is deprecated.
-      It will be removed after Science Pipelines release 23.
-
 .. option:: --dataset <dataset_package>
 
    **Input dataset package.**

doc/lsst.ap.verify/running.rst

@@ -9,7 +9,6 @@ Running ap_verify from the command line
 #######################################
 
 :command:`ap_verify.py` is a Python script designed to be run on both developer machines and verification servers.
-While :command:`ap_verify.py` is not a :doc:`command-line task</modules/lsst.pipe.base/index>`, the command-line interface is designed to resemble that of command-line tasks where practical.
 This page describes the most common options used to run ``ap_verify``.
 For more details, see the :doc:`command-line-reference` or run :option:`ap_verify.py -h`.
 

python/lsst/ap/verify/pipeline_driver.py

@@ -54,8 +54,7 @@ def __init__(self):
         argparse.ArgumentParser.__init__(self, add_help=False)
         # namespace.dataIds will always be a list of 0 or more nonempty strings, regardless of inputs.
         # TODO: in Python 3.8+, action='extend' handles nargs='?' more naturally than 'append'.
-        self.add_argument('--id', '-d', '--data-query', dest='dataIds',
-                          action=self.AppendOptional, nargs='?', default=[],
+        self.add_argument('-d', '--data-query', dest='dataIds', action='append', default=[],
                           help='An identifier for the data to process.')
         self.add_argument("-p", "--pipeline", default=None,
                           help="A custom version of the ap_verify pipeline (e.g., with different metrics). "
@@ -70,20 +69,6 @@ def __init__(self):
                           help="Run the pipeline with a new run collection, "
                                "even if one already exists.")
 
-    class AppendOptional(argparse.Action):
-        """A variant of the built-in "append" action that ignores None values
-        instead of appending them.
-        """
-        # This class can't safely inherit from the built-in "append" action
-        # because there is no public class that implements it.
-        def __call__(self, parser, namespace, values, option_string=None):
-            if values is not None:
-                try:
-                    allValues = getattr(namespace, self.dest)
-                    allValues.append(values)
-                except AttributeError:
-                    setattr(namespace, self.dest, [values])
-
 
 def runApPipeGen3(workspace, parsedCmdLine, processes=1):
     """Run `ap_pipe` on this object's dataset.

tests/test_args.py

@@ -56,7 +56,7 @@ def _parseString(self, commandLine, parser=None):
     def testMissingMain(self):
         """Verify that a command line consisting missing required arguments is rejected.
         """
-        args = '--id "visit=54123" --output tests/output/foo'
+        args = '--data-query "visit=54123" --output tests/output/foo'
         with self.assertRaises(SystemExit):
             self._parseString(args)
 
@@ -88,46 +88,38 @@ def testMinimumIngest(self):
 
     def testDataId(self):
         """Verify that a command line consisting only of required arguments and
-        --id parses correctly.
+        --data-query parses correctly.
         """
-        args = '--dataset %s --output tests/output/foo --id "visit=54123" --id "filter=x"' \
-            % CommandLineTestSuite.testDataset
-        parsed = self._parseString(args)
-        self.assertEqual(parsed.dataIds, ["visit=54123", "filter=x"])
-
-    def testMixedDataId(self):
-        """Verify that a command line with both --id and --data-query parses correctly.
-        """
-        args = '--dataset %s --output tests/output/foo --id "visit=54123" -d "filter=x"' \
+        args = '--dataset %s --output tests/output/foo --data-query "visit=54123" --data-query "filter=x"' \
             % CommandLineTestSuite.testDataset
         parsed = self._parseString(args)
         self.assertEqual(parsed.dataIds, ["visit=54123", "filter=x"])
 
     def testEmptyDataId(self):
-        """Test that an --id argument may be not followed by a data ID.
+        """Test that an --data-query argument must be followed by a data ID.
         """
         minArgs = f'--dataset {self.testDataset} --output tests/output/foo'
 
-        # Nothing after --id
-        parsed = self._parseString(minArgs + ' --id')
-        self.assertEqual(parsed.dataIds, [])
+        # Nothing after --data-query
+        with self.assertRaises(SystemExit):
+            self._parseString(minArgs + ' --data-query')
 
-        # --dataset immediately after --id
-        parsed = self._parseString('--id ' + minArgs)
-        self.assertEqual(parsed.dataIds, [])
+        # --dataset immediately after --data-query
+        with self.assertRaises(SystemExit):
+            self._parseString('--data-query ' + minArgs)
 
     def testBadKeyMain(self):
         """Verify that a command line with unsupported arguments is rejected.
         """
-        args = '--dataset %s --output tests/output/foo --id "visit=54123" --clobber' \
+        args = '--dataset %s --output tests/output/foo --data-query "visit=54123" --clobber' \
             % CommandLineTestSuite.testDataset
         with self.assertRaises(SystemExit):
             self._parseString(args)
 
     def testBadKeyIngest(self):
         """Verify that a command line with unsupported arguments is rejected.
         """
-        args = '--dataset %s --output tests/output/foo --id "visit=54123"' \
+        args = '--dataset %s --output tests/output/foo --data-query "visit=54123"' \
             % CommandLineTestSuite.testDataset
         with self.assertRaises(SystemExit):
             self._parseString(args, ap_verify._IngestOnlyParser())