Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-23503: Deal with / in dataId values in file templates #231

Merged
merged 2 commits into from
Feb 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/datastores/posixDatastore.yaml
Expand Up @@ -8,5 +8,5 @@ datastore:
# valid_first and valid_last here are YYYYMMDD; we assume we'll switch to
# MJD (DM-15890) before we need more than day resolution, since that's all
# Gen2 has.
default: "{collection}/{datasetType}.{component:?}/{tract:?}/{patch:?}/{label:?}/{abstract_filter:?}/{subfilter:?}/{physical_filter:?}/{visit:?}/{datasetType}_{component:?}_{tract:?}_{patch:?}_{label:?}_{abstract_filter:?}_{physical_filter:?}_{calibration_label:?}_{visit:?}_{exposure:?}_{detector:?}_{instrument:?}_{skymap:?}_{skypix:?}_{run}"
default: "{collection:/}/{datasetType}.{component:?}/{tract:?}/{patch:?}/{label:?}/{abstract_filter:?}/{subfilter:?}/{physical_filter:?}/{visit:?}/{datasetType}_{component:?}_{tract:?}_{patch:?}_{label:?}_{abstract_filter:?}_{physical_filter:?}_{calibration_label:?}_{visit:?}_{exposure:?}_{detector:?}_{instrument:?}_{skymap:?}_{skypix:?}_{run}"
formatters: !include formatters.yaml
2 changes: 1 addition & 1 deletion config/datastores/s3Datastore.yaml
Expand Up @@ -8,5 +8,5 @@ datastore:
# valid_first and valid_last here are YYYYMMDD; we assume we'll switch to
# MJD (DM-15890) before we need more than day resolution, since that's all
# Gen2 has.
default: "{collection}/{datasetType}.{component:?}/{tract:?}/{patch:?}/{label:?}/{abstract_filter:?}/{subfilter:?}/{physical_filter:?}/{visit:?}/{datasetType}_{component:?}_{tract:?}_{patch:?}_{label:?}_{abstract_filter:?}_{physical_filter:?}_{calibration_label:?}_{visit:?}_{exposure:?}_{detector:?}_{instrument:?}_{skymap:?}_{skypix:?}_{run}"
default: "{collection:/}/{datasetType}.{component:?}/{tract:?}/{patch:?}/{label:?}/{abstract_filter:?}/{subfilter:?}/{physical_filter:?}/{visit:?}/{datasetType}_{component:?}_{tract:?}_{patch:?}_{label:?}_{abstract_filter:?}_{physical_filter:?}_{calibration_label:?}_{visit:?}_{exposure:?}_{detector:?}_{instrument:?}_{skymap:?}_{skypix:?}_{run}"
formatters: !include formatters.yaml
17 changes: 17 additions & 0 deletions python/lsst/daf/butler/core/fileTemplates.py
Expand Up @@ -303,6 +303,11 @@ class FileTemplate:
specification. This indicates that a field is optional. If that
Dimension is missing the field, along with the text before the field,
unless it is a path separator, will be removed from the output path.

By default any "/" in a dataId value will be replaced by "_" to prevent
unexpected directories being created in the path. If the "/" should be
retained then a special "/" format specifier can be included in the
template.
"""

mandatoryFields = {"collection", "run"}
Expand Down Expand Up @@ -481,6 +486,18 @@ def format(self, ref):
raise KeyError(f"'{field_name}' requested in template via '{self.template}' "
"but not defined and not optional")

# Handle "/" in values since we do not want to be surprised by
# unexpected directories turning up
replace_slash = True
if "/" in format_spec:
# Remove the non-standard character from the spec
format_spec = format_spec.replace("/", "")
replace_slash = False

if isinstance(value, str):
if replace_slash:
value = value.replace("/", "_")

# Now use standard formatting
output = output + literal + format(value, format_spec)

Expand Down
18 changes: 16 additions & 2 deletions tests/test_templates.py
Expand Up @@ -35,13 +35,13 @@ class TestFileTemplates(unittest.TestCase):
"""Test creation of paths from templates."""

def makeDatasetRef(self, datasetTypeName, dataId=None, storageClassName="DefaultStorageClass",
conform=True):
run="run2", conform=True):
"""Make a simple DatasetRef"""
if dataId is None:
dataId = self.dataId
datasetType = DatasetType(datasetTypeName, DimensionGraph(self.universe, names=dataId.keys()),
StorageClass(storageClassName))
return DatasetRef(datasetType, dataId, id=1, run="run2", conform=conform)
return DatasetRef(datasetType, dataId, id=1, run=run, conform=conform)

def setUp(self):
self.universe = DimensionUniverse()
Expand All @@ -62,6 +62,20 @@ def testBasic(self):
"run2/calexp/00052/U-trail",
self.makeDatasetRef("calexp", conform=False))

tmplstr = "{collection}/{datasetType}/{visit:05d}/{physical_filter}-trail-{run}"
self.assertTemplate(tmplstr,
"run2/calexp/00052/U-trail-run2",
self.makeDatasetRef("calexp", conform=False))
self.assertTemplate(tmplstr,
"run_2/calexp/00052/U-trail-run_2",
self.makeDatasetRef("calexp", run="run/2", conform=False))

# Retain any "/" in collection
tmplstr = "{collection:/}/{datasetType}/{visit:05d}/{physical_filter}-trail-{run}"
self.assertTemplate(tmplstr,
"run/2/calexp/00052/U-trail-run_2",
self.makeDatasetRef("calexp", run="run/2", conform=False))

with self.assertRaises(FileTemplateValidationError):
FileTemplate("no fields at all")

Expand Down