Skip to content

Commit

Permalink
Add default value to query classes to allow usage to be detected
Browse files Browse the repository at this point in the history
The components parameter is deprecated. Warn if False is specified
explicitly in addition to raising if any other value is used.
  • Loading branch information
timj committed Jan 9, 2024
1 parent 4d10137 commit fa2c67b
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 30 deletions.
9 changes: 5 additions & 4 deletions python/lsst/daf/butler/_registry_shim.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
from .registry._collection_type import CollectionType
from .registry._defaults import RegistryDefaults
from .registry.queries import DataCoordinateQueryResults, DatasetQueryResults, DimensionRecordQueryResults
from .utils import _DefaultMarker, _Marker

if TYPE_CHECKING:
from .direct_butler import DirectButler
Expand Down Expand Up @@ -281,7 +282,7 @@ def queryDatasetTypes(
self,
expression: Any = ...,
*,
components: bool = False,
components: bool | _Marker = _DefaultMarker,
missing: list[str] | None = None,
) -> Iterable[DatasetType]:
# Docstring inherited from a base class.
Expand Down Expand Up @@ -309,7 +310,7 @@ def queryDatasets(
dataId: DataId | None = None,
where: str = "",
findFirst: bool = False,
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand Down Expand Up @@ -337,7 +338,7 @@ def queryDataIds(
datasets: Any = None,
collections: CollectionArgType | None = None,
where: str = "",
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand All @@ -363,7 +364,7 @@ def queryDimensionRecords(
datasets: Any = None,
collections: CollectionArgType | None = None,
where: str = "",
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand Down
79 changes: 54 additions & 25 deletions python/lsst/daf/butler/registry/sql_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import sqlalchemy
from lsst.daf.relation import LeafRelation, Relation
from lsst.resources import ResourcePathExpression
from lsst.utils.introspection import find_outside_stacklevel
from lsst.utils.iteration import ensure_iterable

from .._column_tags import DatasetColumnTag
Expand Down Expand Up @@ -85,7 +86,7 @@
from ..registry.interfaces import ChainedCollectionRecord, ReadOnlyDatabaseError, RunRecord
from ..registry.managers import RegistryManagerInstances, RegistryManagerTypes
from ..registry.wildcards import CollectionWildcard, DatasetTypeWildcard
from ..utils import transactional
from ..utils import transactional, _DefaultMarker, _Marker

if TYPE_CHECKING:
from .._butler_config import ButlerConfig
Expand Down Expand Up @@ -1703,7 +1704,7 @@ def queryDatasetTypes(
self,
expression: Any = ...,
*,
components: bool = False,
components: bool | _Marker = _DefaultMarker,
missing: list[str] | None = None,
) -> Iterable[DatasetType]:
"""Iterate over the dataset types whose names match an expression.
Expand Down Expand Up @@ -1735,11 +1736,18 @@ def queryDatasetTypes(
lsst.daf.butler.registry.DatasetTypeExpressionError
Raised when ``expression`` is invalid.
"""
if components is not False:
raise DatasetTypeError(
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
if components is not _DefaultMarker:
if components is not False:
raise DatasetTypeError(

Check warning on line 1741 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L1741

Added line #L1741 was not covered by tests
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
else:
warnings.warn(
"The components parameter is ignored. It will be removed after v27.",
category=FutureWarning,
stacklevel=find_outside_stacklevel("lsst.daf.butler"),
)
wildcard = DatasetTypeWildcard.from_expression(expression)
return self._managers.datasets.resolve_wildcard(wildcard, missing=missing)

Expand Down Expand Up @@ -1966,7 +1974,7 @@ def queryDatasets(
dataId: DataId | None = None,
where: str = "",
findFirst: bool = False,
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand Down Expand Up @@ -2068,11 +2076,18 @@ def queryDatasets(
query), and then use multiple (generally much simpler) calls to
`queryDatasets` with the returned data IDs passed as constraints.
"""
if components is not False:
raise DatasetTypeError(
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
if components is not _DefaultMarker:
if components is not False:
raise DatasetTypeError(

Check warning on line 2081 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2081

Added line #L2081 was not covered by tests
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
else:
warnings.warn(

Check warning on line 2086 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2086

Added line #L2086 was not covered by tests
"The components parameter is ignored. It will be removed after v27.",
category=FutureWarning,
stacklevel=find_outside_stacklevel("lsst.daf.butler"),
)
doomed_by: list[str] = []
data_id = self._standardize_query_data_id_args(dataId, doomed_by=doomed_by, **kwargs)
resolved_dataset_types, collection_wildcard = self._standardize_query_dataset_args(
Expand Down Expand Up @@ -2138,7 +2153,7 @@ def queryDataIds(
datasets: Any = None,
collections: CollectionArgType | None = None,
where: str = "",
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand Down Expand Up @@ -2231,11 +2246,18 @@ def queryDataIds(
lsst.daf.butler.registry.UserExpressionError
Raised when ``where`` expression is invalid.
"""
if components is not False:
raise DatasetTypeError(
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
if components is not _DefaultMarker:
if components is not False:
raise DatasetTypeError(

Check warning on line 2251 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2251

Added line #L2251 was not covered by tests
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
else:
warnings.warn(

Check warning on line 2256 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2256

Added line #L2256 was not covered by tests
"The components parameter is ignored. It will be removed after v27.",
category=FutureWarning,
stacklevel=find_outside_stacklevel("lsst.daf.butler"),
)
requested_dimensions = self.dimensions.conform(dimensions)
doomed_by: list[str] = []
data_id = self._standardize_query_data_id_args(dataId, doomed_by=doomed_by, **kwargs)
Expand Down Expand Up @@ -2269,7 +2291,7 @@ def queryDimensionRecords(
datasets: Any = None,
collections: CollectionArgType | None = None,
where: str = "",
components: bool = False,
components: bool | _Marker = _DefaultMarker,
bind: Mapping[str, Any] | None = None,
check: bool = True,
**kwargs: Any,
Expand Down Expand Up @@ -2344,11 +2366,18 @@ def queryDimensionRecords(
lsst.daf.butler.registry.UserExpressionError
Raised when ``where`` expression is invalid.
"""
if components is not False:
raise DatasetTypeError(
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
if components is not _DefaultMarker:
if components is not False:
raise DatasetTypeError(

Check warning on line 2371 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2371

Added line #L2371 was not covered by tests
"Dataset component queries are no longer supported by Registry. Use "
"DatasetType methods to obtain components from parent dataset types instead."
)
else:
warnings.warn(

Check warning on line 2376 in python/lsst/daf/butler/registry/sql_registry.py

View check run for this annotation

Codecov / codecov/patch

python/lsst/daf/butler/registry/sql_registry.py#L2376

Added line #L2376 was not covered by tests
"The components parameter is ignored. It will be removed after v27.",
category=FutureWarning,
stacklevel=find_outside_stacklevel("lsst.daf.butler"),
)
if not isinstance(element, DimensionElement):
try:
element = self.dimensions[element]
Expand Down
12 changes: 12 additions & 0 deletions python/lsst/daf/butler/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,15 @@ def globToRegex(expressions: str | EllipsisType | None | list[str]) -> list[str
res = e
results.append(res)
return results


class _Marker:
"""Private class to use as a default value when you want to know that
a default parameter has been over-ridden.
"""


_DefaultMarker = _Marker()
"""Default value to give to a parameter when you want to know if the value
has been over-ridden.
"""
3 changes: 2 additions & 1 deletion tests/test_testRepo.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ def testAddDataIdValue(self):
def testAddDatasetType(self):
# 1 for StructuredDataNoComponents, 1 for StructuredData (components
# not included).
self.assertEqual(len(list(self.butler.registry.queryDatasetTypes(components=False))), 2)
with self.assertWarns(FutureWarning):
self.assertEqual(len(list(self.butler.registry.queryDatasetTypes(components=False))), 2)

# Testing the DatasetType objects is not practical, because all tests
# need a DimensionUniverse. So just check that we have the dataset
Expand Down

0 comments on commit fa2c67b

Please sign in to comment.