DM-46297: require label to be provided to write-curated-calibrations

doc/changes/DM-46297.api.md

@@ -0,0 +1,3 @@
+The `butler write-curated-calibrations` command now requires at least one "label" to be included for the collection name.
+
+This prevents the common mistake of setting up `<instrument>/calib` as a `CALIBRATION` collection rather than a more maintainable `CHAINED` collection.

python/lsst/obs/base/_instrument.py

@@ -581,6 +581,15 @@ def group_name_to_group_id(cls, group_name: str) -> int:
         cleaned = re.sub(r"\D", "", group_name)
         return int(cleaned)
 
+    def get_curated_calibration_labels(self) -> list[str]:
+        """Return appropriate labels (pieces of a collection name) for a
+        collection populated by `writeCuratedCalibrations`.
+
+        If this returns an empty list (as the default implementation does),
+        the user will be required to provide a label.
+        """
+        return []
+
 
 def makeExposureRecordFromObsInfo(
     obsInfo: ObservationInfo, universe: DimensionUniverse, **kwargs: Any

python/lsst/obs/base/cli/cmd/commands.py

@@ -36,7 +36,7 @@
 from lsst.pipe.base.cli.opt import instrument_argument
 
 from ... import script
-from ..opt import failfast_option
+from ..opt import failfast_option, labels_argument
 
 # regular expression that can be used to find supported fits file extensions.
 fits_re = r"\.fit[s]?\b"
@@ -168,6 +168,7 @@ def ingest_raws(*args, **kwargs):
 @click.command(short_help="Add an instrument's curated calibrations.", cls=ButlerCommand)
 @repo_argument(required=True)
 @instrument_argument(required=True)
+@labels_argument()
 @click.option(
     "--collection",
     required=False,
@@ -179,10 +180,23 @@ def ingest_raws(*args, **kwargs):
     multiple=True,
     help=(
         "Extra strings to include (with automatic delimiters) in all RUN collection names, "
-        "as well as the calibration collection name if it is not provided via --collection."
+        "as well as the calibration collection name if it is not provided via --collection. "
+        "May be provided as a positional argument instead of or in addition to this option, "
+        "as long as at least one label is provided.  Positional-argument labels appear before "
+        "those provided by this option."
+    ),
+)
+@click.option(
+    "--prefix",
+    required=False,
+    help=(
+        "Prefix for the collection name.  Default is the instrument name. "
+        "This is ignored if --collection is passed."
     ),
 )
 @options_file_option()
-def write_curated_calibrations(*args, **kwargs):
+def write_curated_calibrations(*, repo, instrument, collection, labels, labels_arg, prefix):
     """Add an instrument's curated calibrations to the data repository."""
-    script.writeCuratedCalibrations(*args, **kwargs)
+    script.writeCuratedCalibrations(
+        repo=repo, instrument=instrument, collection=collection, labels=labels_arg + labels, prefix=prefix
+    )

python/lsst/obs/base/cli/opt/options.py

@@ -19,7 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from lsst.daf.butler.cli.utils import MWOptionDecorator
+from lsst.daf.butler.cli.utils import MWArgumentDecorator, MWOptionDecorator
 
 failfast_option = MWOptionDecorator(
     "--fail-fast",
@@ -29,3 +29,10 @@
     ),
     is_flag=True,
 )
+
+labels_argument = MWArgumentDecorator(
+    "labels_arg",
+    metavar="LBL",
+    nargs=-1,
+    help="LBL: One or more extra strings to include in the collection name (see --label).",
+)

python/lsst/obs/base/ingest_tests.py

@@ -319,7 +319,9 @@ def _writeCuratedCalibrations(self):
         to write curated calibrations.
         """
         runner = LogCliRunner()
-        result = runner.invoke(butlerCli, ["write-curated-calibrations", self.root, self.instrumentName])
+        result = runner.invoke(
+            butlerCli, ["write-curated-calibrations", self.root, self.instrumentName, "test"]
+        )
         self.assertEqual(result.exit_code, 0, f"output: {result.output} exception: {result.exception}")
 
     def testLink(self):
@@ -419,7 +421,7 @@ def testWriteCuratedCalibrations(self):
             raise unittest.SkipTest("Class requests disabling of writeCuratedCalibrations test")
 
         butler = Butler(self.root, writeable=False)
-        collection = self.instrumentClass().makeCalibrationCollectionName()
+        collection = self.instrumentClass().makeCalibrationCollectionName("test")
 
         # Trying to load a camera with a data ID not known to the registry
         # is an error, because we can't get any temporal information.

python/lsst/obs/base/script/writeCuratedCalibrations.py

@@ -27,7 +27,7 @@
 log = logging.getLogger(__name__)
 
 
-def writeCuratedCalibrations(repo, instrument, collection, labels):
+def writeCuratedCalibrations(repo, instrument, collection, labels, prefix=None):
     """Add an instrument's curated calibrations to the data repository.
 
     Parameters
@@ -45,6 +45,9 @@ def writeCuratedCalibrations(repo, instrument, collection, labels):
         Extra strings to include in the names of collections that datasets are
         inserted directly into, and if ``collection`` is `None`, the automatic
         calibration collection name as well.
+    prefix : `str`, optional
+        Prefix for the collection name to use instead of the instrument name.
+        Ignored if ``collection`` is passed.
 
     Raises
     ------
@@ -56,5 +59,9 @@ def writeCuratedCalibrations(repo, instrument, collection, labels):
         `lsst.obs.base.Instrument`.
     """
     butler = Butler(repo, writeable=True)
-    instr = Instrument.from_string(instrument, butler.registry)
+    instr = Instrument.from_string(instrument, butler.registry, collection_prefix=prefix)
+    if collection is None and not labels:
+        labels = instr.get_curated_calibration_labels()
+        if not labels:
+            raise ValueError("At least one label or --collection must be provided.")
     instr.writeCuratedCalibrations(butler, collection=collection, labels=labels)

tests/test_cliCmdWriteCuratedCalibrations.py

@@ -45,7 +45,7 @@ def test_repoBasic(self):
         """Test the most basic required arguments."""
         self.run_test(
             ["write-curated-calibrations", "here", "a.b.c", "--collection", "foo"],
-            self.makeExpected(repo="here", instrument="a.b.c", collection="foo", labels=()),
+            self.makeExpected(repo="here", instrument="a.b.c", collection="foo", labels=(), prefix=None),
         )
 
     def test_missing(self):