Merge branch 'tickets/DM-34261' into main

doc/changes/DM-34261.bugfix.rst

@@ -0,0 +1 @@
+The `click.Path` API changed, change from ordered arguments to keyword arguments when calling it.

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

@@ -289,7 +289,7 @@ def _raiseIfDuplicateCommands(commands):
             if len(packages) > 1:
                 msg += f"Command '{command}' exists in packages {', '.join(packages)}. "
         if msg:
-            raise click.ClickException(msg + "Duplicate commands are not supported, aborting.")
+            raise click.ClickException(message=msg + "Duplicate commands are not supported, aborting.")
 
 
 class ButlerCLI(LoaderCLI):

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

@@ -68,7 +68,7 @@ def remove_collections(**kwargs):
             result.runsTable.pprint_all(align="<")
             print()
         if canRemoveCollections:
-            doContinue = click.confirm("Continue?", default=False)
+            doContinue = click.confirm(text="Continue?", default=False)
     if doContinue:
         result.onConfirmation()
         if confirm:

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

@@ -111,7 +111,7 @@ def remove_runs(context, confirm, force, **kwargs):
         return
     if confirm:
         _print_remove(True, result.runs, result.datasets)
-        doContinue = click.confirm("Continue?", default=False)
+        doContinue = click.confirm(text="Continue?", default=False)
         if doContinue:
             result.onConfirmation()
             print(removedRunsMsg)

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

@@ -93,7 +93,7 @@ def associate(**kwargs):
     "datasets.  If this is not an absolute path, does not exist in the current working "
     "directory, and --dir is provided, it is assumed to be in that directory.  Defaults "
     'to "export.yaml".',
-    type=click.File("r"),
+    type=click.File(mode="r"),
 )
 @click.option(
     "--skip-dimensions",
@@ -159,7 +159,7 @@ def create(*args, **kwargs):
 @click.option(
     "--file",
     "outfile",
-    type=click.File("w"),
+    type=click.File(mode="w"),
     default="-",
     help="Print the (possibly-expanded) configuration for a repository to a file, or to stdout "
     "by default.",
@@ -234,7 +234,7 @@ def prune_collection(**kwargs):
     if result.confirm:
         print("The following collections will be removed:")
         result.removeTable.pprint_all(align="<")
-        doContinue = click.confirm("Continue?", default=False)
+        doContinue = click.confirm(text="Continue?", default=False)
     else:
         doContinue = True
     if doContinue:
@@ -379,20 +379,19 @@ def prune_datasets(**kwargs):
     quiet = kwargs.pop("quiet", False)
     if quiet:
         if kwargs["dry_run"]:
-            raise click.ClickException(pruneDatasets_errQuietWithDryRun)
+            raise click.ClickException(message=pruneDatasets_errQuietWithDryRun)
         kwargs["confirm"] = False
 
     result = script.pruneDatasets(**kwargs)
 
     if result.errPurgeAndDisassociate:
-        raise click.ClickException(pruneDatasets_errPurgeAndDisassociate)
-        return
+        raise click.ClickException(message=pruneDatasets_errPurgeAndDisassociate)
     if result.errNoCollectionRestriction:
-        raise click.ClickException(pruneDatasets_errNoCollectionRestriction)
+        raise click.ClickException(message=pruneDatasets_errNoCollectionRestriction)
     if result.errPruneOnNotRun:
-        raise click.ClickException(pruneDatasets_errPruneOnNotRun.format(**result.errDict))
+        raise click.ClickException(message=pruneDatasets_errPruneOnNotRun.format(**result.errDict))
     if result.errNoOp:
-        raise click.ClickException(pruneDatasets_errNoOp)
+        raise click.ClickException(message=pruneDatasets_errNoOp)
     if result.dryRun:
         if result.action["disassociate"] and result.action["unstore"]:
             msg = pruneDatasets_wouldDisassociateAndRemoveMsg
@@ -409,7 +408,7 @@ def prune_datasets(**kwargs):
             return
         print(pruneDatasets_willRemoveMsg)
         printAstropyTables(result.tables)
-        doContinue = click.confirm(pruneDatasets_askContinueMsg, default=False)
+        doContinue = click.confirm(text=pruneDatasets_askContinueMsg, default=False)
         if doContinue:
             result.onConfirmation()
             print(pruneDatasets_didRemoveAforementioned)
@@ -456,7 +455,7 @@ def prune_datasets(**kwargs):
     # the FLATTEN text.
     callback=to_upper,
     type=click.Choice(
-        ("TABLE", "INVERSE-TABLE", "TREE", "INVERSE-TREE", "FLATTEN"),
+        choices=("TABLE", "INVERSE-TABLE", "TREE", "INVERSE-TREE", "FLATTEN"),
         case_sensitive=False,
     ),
 )

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

@@ -53,7 +53,7 @@ def makeCollectionTypes(context, param, value):
     callback=CollectionTypeCallback.makeCollectionTypes,
     multiple=True,
     help="If provided, only list collections of this type.",
-    type=click.Choice(CollectionTypeCallback.collectionTypes, case_sensitive=False),
+    type=click.Choice(choices=CollectionTypeCallback.collectionTypes, case_sensitive=False),
 )
 
 
@@ -124,7 +124,7 @@ def makeCollectionTypes(context, param, value):
     "--log-level",
     callback=partial(
         split_kv,
-        choice=click.Choice(logLevelChoices, case_sensitive=False),
+        choice=click.Choice(choices=logLevelChoices, case_sensitive=False),
         normalize=True,
         unseparated_okay=True,
         add_to_default=True,
@@ -209,7 +209,8 @@ def makeCollectionTypes(context, param, value):
     default="auto",  # set to `None` if using `required=True`
     help="The external data transfer mode.",
     type=click.Choice(
-        ["auto", "link", "symlink", "hardlink", "copy", "move", "relsymlink", "direct"], case_sensitive=False
+        choices=["auto", "link", "symlink", "hardlink", "copy", "move", "relsymlink", "direct"],
+        case_sensitive=False,
     ),
 )
 

python/lsst/daf/butler/cli/progress.py

@@ -77,4 +77,4 @@ def get_progress_bar(
         self, iterable: Optional[Iterable[_T]], desc: Optional[str], total: Optional[int], level: int
     ) -> ContextManager[ProgressBar[_T]]:
         # Docstring inherited.
-        return click.progressbar(iterable, length=total, label=desc, **self._kwargs)
+        return click.progressbar(iterable=iterable, length=total, label=desc, **self._kwargs)

python/lsst/daf/butler/cli/utils.py

@@ -375,7 +375,9 @@ def get(self):
     elif return_type is tuple:
         ret = RetTuple()
     else:
-        raise click.ClickException(f"Internal error: invalid return type '{return_type}' for split_kv.")
+        raise click.ClickException(
+            message=f"Internal error: invalid return type '{return_type}' for split_kv."
+        )
     if multiple:
         vals = split_commas(context, param, vals)
     for val in ensure_iterable(vals):
@@ -390,7 +392,7 @@ def get(self):
                     choice(v)  # will raise if val is an invalid choice
             except ValueError:
                 raise click.ClickException(
-                    f"Could not parse key-value pair '{val}' using separator '{separator}', "
+                    message=f"Could not parse key-value pair '{val}' using separator '{separator}', "
                     f"with multiple values {'allowed' if multiple else 'not allowed'}."
                 )
             ret.add(k, norm(v))
@@ -505,7 +507,16 @@ def __init__(
         self.mustNotExist = exists is False
         if exists is None:
             exists = False
-        super().__init__(exists, file_okay, dir_okay, writable, readable, resolve_path, allow_dash, path_type)
+        super().__init__(
+            exists=exists,
+            file_okay=file_okay,
+            dir_okay=dir_okay,
+            writable=writable,
+            readable=readable,
+            resolve_path=resolve_path,
+            allow_dash=allow_dash,
+            path_type=path_type,
+        )
 
     def convert(self, value, param, ctx):
         """Called by click.ParamType to "convert values through types".
@@ -802,7 +813,11 @@ def _name_for_option(ctx: click.Context, option: str) -> str:
                     continue
                 overrides[name] = overrides.pop(option)
         except Exception as e:
-            raise click.BadOptionUsage(param.name, f"Error reading overrides file: {e}", ctx)
+            raise click.BadOptionUsage(
+                option_name=param.name,
+                message=f"Error reading overrides file: {e}",
+                ctx=ctx,
+            )
         # Override the defaults for this subcommand
         ctx.default_map.update(overrides)
     return

python/lsst/daf/butler/tests/cliLogTestBase.py

@@ -94,7 +94,9 @@ def command_log_settings_test(
         )
     for expected, actual, name in logLevels:
         if expected != actual:
-            raise (click.ClickException(f"expected {name} level to be {expected!r}, actual:{actual!r}"))
+            raise (
+                click.ClickException(message=f"expected {name} level to be {expected!r}, actual:{actual!r}")
+            )
 
 
 class CliLogTestBase:

tests/test_cliCmdPruneDatasets.py

@@ -37,6 +37,7 @@
     pruneDatasets_didRemoveAforementioned,
     pruneDatasets_didRemoveMsg,
     pruneDatasets_errNoCollectionRestriction,
+    pruneDatasets_errNoOp,
     pruneDatasets_errPruneOnNotRun,
     pruneDatasets_errPurgeAndDisassociate,
     pruneDatasets_errQuietWithDryRun,
@@ -368,6 +369,18 @@ def test_purgeWithDisassociate(self):
             exPruneDatasetsExitCode=1,
         )
 
+    def test_purgeNoOp(self):
+        """Verify there is an error when none of --purge, --unstore, or
+        --disassociate are passed."""
+        self.run_test(
+            cliArgs=[],
+            exPruneDatasetsCallArgs=None,
+            exQueryDatasetsCallArgs=None,  # should not make it far enough to call this.
+            exGetTablesCalled=False,  # ...or this.
+            exMsgs=(pruneDatasets_errNoOp,),
+            exPruneDatasetsExitCode=1,
+        )
+
     @patch.object(
         lsst.daf.butler.registries.sql.SqlRegistry,
         "getCollectionType",

tests/test_cliPluginLoader.py

@@ -36,7 +36,7 @@
 
 @click.command()
 def command_test():
-    click.echo("test command")
+    click.echo(message="test command")
 
 
 @contextmanager

tests/test_cliUtilSplitKv.py

@@ -124,6 +124,15 @@ def test_reverseKv(self):
             {"1": "first", "second": "key"},
         )
 
+    def test_invalidResultType(self):
+        with self.assertRaises(click.ClickException):
+            split_kv(
+                "context",
+                "param",
+                "first=1,second=2",
+                return_type=set,
+            )
+
 
 class SplitKvCmdTestCase(unittest.TestCase):
     """Tests using split_kv with a command."""
@@ -170,7 +179,7 @@ def test_choice(self):
             callback=partial(
                 split_kv,
                 unseparated_okay=True,
-                choice=click.Choice(choices, case_sensitive=False),
+                choice=click.Choice(choices=choices, case_sensitive=False),
                 normalize=True,
             ),
         )