DM-24560: make 'repo' an argument, by convention always the first.

python/lsst/daf/butler/cli/cmd/config_dump.py

@@ -22,12 +22,11 @@
 import click
 
 from ... import ButlerConfig
-from ..opt import repo_option
+from ..opt import repo_argument
 
 
 @click.command()
-@repo_option(required=True, help="filesystem path for an existing Butler repository or path to config "
-             "file.")
+@repo_argument(required=True)
 @click.option("--subset", "-s", type=str,
               help="Subset of a configuration to report. This can be any key in the hierarchy such as "
               "'.datastore.root' where the leading '.' specified the delimiter for the hierarchy.")

python/lsst/daf/butler/cli/cmd/config_validate.py

@@ -22,13 +22,13 @@
 import click
 
 from ... import Butler, ValidationError
-from ..opt import dataset_type_option, repo_option
+from ..opt import dataset_type_option, repo_argument
 from ..utils import split_commas
 
 
 @click.command()
 @click.pass_context
-@repo_option(required=True)
+@repo_argument(required=True)
 @click.option("--quiet", "-q", is_flag=True, help="Do not report individual failures.")
 @dataset_type_option(help="Specific DatasetType(s) to validate.")
 @click.option("--ignore", "-i", type=str, multiple=True, callback=split_commas,

python/lsst/daf/butler/cli/cmd/create.py

@@ -22,12 +22,11 @@
 import click
 
 from ... import Butler, Config
-from ..opt import repo_option
+from ..opt import repo_argument
 
 
 @click.command()
-@repo_option(required=True, help="The filesystem path for the new repository. Will be created if it does not "
-             "exist.")
+@repo_argument(help=repo_argument.will_create_repo)
 @click.option("--config", "-c", help="Path to an existing YAML config file to apply (on top of defaults).")
 @click.option("--standalone", is_flag=True, help="Include all defaults in the config file in the repo, "
               "insulating the repo from changes in package defaults.")

python/lsst/daf/butler/cli/opt/__init__.py

@@ -20,5 +20,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from .dataset_type import dataset_type_option
-from .repo import repo_option
+from .repo import repo_argument
 from .run import run_option
+

python/lsst/daf/butler/cli/opt/repo.py

@@ -23,13 +23,34 @@
 import click
 
 
-class repo_option:  # noqa: N801
-    def __init__(self, required=False, help=None):
-        self.help = help if help is not None else """Path to an existing data repository root or configuration
-                    file."""
+class repo_argument:  # noqa: N801
+    """Decorator to add a repo argument to a click command.
+
+    Parameters
+    ----------
+    required : bool, optional
+        Indicates if the caller must pass this argument to the command, by
+        default True.
+    help : str, optional
+        The help text for this argument to append to the command's help text.
+        If None or '' then nothing will be appended to the help text (in which
+        case the command should document this argument directly in its help
+        text). By default the value of repo_argument.existing_repo.
+    """
+
+    will_create_repo = "REPO is the URI or path to the new repository. " \
+                       "Will be created if it does not exist."
+    existing_repo = "REPO is the URI or path to an existing data repository root " \
+                    "or configuration file."
+
+    def __init__(self, required=False, help=existing_repo):
         self.required = required
+        self.helpText = help
 
     def __call__(self, f):
-        return click.option('--repo',
-                            required=self.required,
-                            help=self.help)(f)
+        if self.helpText:
+            # Modify the passed-in fucntions's doc string, which is used to
+            # generate the Click Command help, to include the argument help
+            # text:
+            f.__doc__ = f"{'' if f.__doc__ is None else f.__doc__}\n\n {self.helpText}"
+        return click.argument("repo", required=self.required)(f)

tests/test_cliOptionRepo.py → tests/test_cliArgumentRepo.py

@@ -26,43 +26,49 @@
 import click.testing
 import unittest
 
-from lsst.daf.butler.cli.opt.repo import repo_option
+from lsst.daf.butler.cli.opt import repo_argument
 
 
 @click.command()
-@repo_option(required=True)
+@repo_argument(required=True)
 def repoRequired(repo):
     pass
 
 
 @click.command()
-@repo_option()  # required default val is False
+@repo_argument(required=False)
 def repoNotRequired(repo):
     pass
 
 
 @click.command()
-@repo_option(help="custom help text")
+@repo_argument(help="REPO custom help text")
 def repoWithHelpText(repo):
     pass
 
 
+@click.command()
+@repo_argument(help=repo_argument.will_create_repo)
+def repoWithWillCreateHelpText(repo):
+    pass
+
+
 class Suite(unittest.TestCase):
 
     def testRequired_provided(self):
         runner = click.testing.CliRunner()
-        result = runner.invoke(repoRequired, ["--repo", "location"])
+        result = runner.invoke(repoRequired, ["location"])
         self.assertEqual(result.exit_code, 0)
 
     def testRequired_notProvided(self):
         runner = click.testing.CliRunner()
         result = runner.invoke(repoRequired)
         self.assertNotEqual(result.exit_code, 0)
-        self.assertIn('Missing option "--repo"', result.output)
+        self.assertIn('Error: Missing argument "REPO"', result.output)
 
     def testNotRequired_provided(self):
         runner = click.testing.CliRunner()
-        result = runner.invoke(repoNotRequired, ["--repo", "location"])
+        result = runner.invoke(repoNotRequired, ["location"])
         self.assertEqual(result.exit_code, 0)
 
     def testNotRequired_notProvided(self):
@@ -75,6 +81,11 @@ def testHelp(self):
         result = runner.invoke(repoWithHelpText, "--help")
         self.assertIn("custom help text", result.output)
 
+    def testWillCreateHelpText(self):
+        runner = click.testing.CliRunner()
+        result = runner.invoke(repoWithWillCreateHelpText, "--help")
+        self.assertIn(repo_argument.will_create_repo, result.output.replace("\n ", ""))
+
 
 if __name__ == "__main__":
     unittest.main()

tests/test_cliCmdConfigDump.py

@@ -41,11 +41,11 @@ def test_stdout(self):
         """Test dumping the config to stdout."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
 
             # test dumping to stdout:
-            result = runner.invoke(butler.cli, ["config-dump", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["config-dump", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             # check for some expected keywords:
             cfg = yaml.safe_load(result.stdout)
@@ -57,9 +57,9 @@ def test_file(self):
         """test dumping the config to a file."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
-            result = runner.invoke(butler.cli, ["config-dump", "--repo", "here", "--file=there"])
+            result = runner.invoke(butler.cli, ["config-dump", "here", "--file=there"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             # check for some expected keywords:
             with open("there", "r") as f:
@@ -72,9 +72,9 @@ def test_subset(self):
         """Test selecting a subset of the config."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
-            result = runner.invoke(butler.cli, ["config-dump", "--repo", "here", "--subset", "datastore"])
+            result = runner.invoke(butler.cli, ["config-dump", "here", "--subset", "datastore"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             cfg = yaml.safe_load(result.stdout)
             # count the keys in the datastore config
@@ -90,10 +90,10 @@ def test_invalidSubset(self):
         """Test selecting a subset key that does not exist in the config."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             # test dumping to stdout:
-            result = runner.invoke(butler.cli, ["config-dump", "--repo", "here", "--subset", "foo"])
+            result = runner.invoke(butler.cli, ["config-dump", "here", "--subset", "foo"])
             self.assertEqual(result.exit_code, 1)
             self.assertEqual(result.output, "Error: foo not found in config at here\n")
 

tests/test_cliCmdConfigValidate.py

@@ -40,21 +40,21 @@ def testConfigValidate(self):
         """Test validating a valid config."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             # verify the just-created repo validates without error
-            result = runner.invoke(butler.cli, ["config-validate", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["config-validate", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             self.assertEqual(result.stdout, "No problems encountered with configuration.\n")
 
     def testConfigValidate_ignore(self):
         """Test the ignore flag"""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             # verify the just-created repo validates without error
-            result = runner.invoke(butler.cli, ["config-validate", "--repo", "here",
+            result = runner.invoke(butler.cli, ["config-validate", "here",
                                    "--ignore", "storageClasses,repoTransferFormats", "-i", "dimensions"])
             self.assertEqual(result.exit_code, 0, result.stdout)
             self.assertEqual(result.stdout, "No problems encountered with configuration.\n")

tests/test_cliPluginLoader.py

@@ -91,7 +91,7 @@ def test_loadAndExecuteLocalCommand(self):
         """Test that a command in daf_butler can be loaded and executed."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "test_repo"])
+            result = runner.invoke(butler.cli, ["create", "test_repo"])
             self.assertEqual(result.exit_code, 0, result.output)
             self.assertTrue(os.path.exists("test_repo"))
 
@@ -127,7 +127,7 @@ def test_listCommands_duplicate(self):
         self.maxDiff = None
         runner = click.testing.CliRunner()
         with duplicate_command_test_env(runner):
-            result = runner.invoke(butler.cli, ["create", "--repo", "test_repo"])
+            result = runner.invoke(butler.cli, ["create", "test_repo"])
             self.assertEqual(result.exit_code, 1, result.output)
             self.assertEqual(result.output, "Error: Command 'create' "
                              "exists in packages lsst.daf.butler.cli.cmd, test_cliPluginLoader. "

tests/test_commandLine.py

@@ -40,14 +40,14 @@ def testCreate(self):
         """Test creating a repostiry."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here"])
+            result = runner.invoke(butler.cli, ["create", "here"])
             self.assertEqual(result.exit_code, 0)
 
     def testCreate_outfile(self):
         """Test creating a repository and specify an outfile location."""
         runner = click.testing.CliRunner()
         with runner.isolated_filesystem():
-            result = runner.invoke(butler.cli, ["create", "--repo", "here", "--outfile=there"])
+            result = runner.invoke(butler.cli, ["create", "here", "--outfile=there"])
             self.assertEqual(result.exit_code, 0)
             self.assertTrue(os.path.exists("there"))  # verify the separate config file was made
             with open("there", "r") as f: