Skip to content

Commit

Permalink
Merge pull request #1096 from lsst/tickets/DM-46599
Browse files Browse the repository at this point in the history
DM-46599: Deprecate old query system behavior
  • Loading branch information
dhirving authored Oct 16, 2024
2 parents 6616209 + 84cf62c commit 3abf212
Show file tree
Hide file tree
Showing 19 changed files with 276 additions and 113 deletions.
19 changes: 19 additions & 0 deletions doc/changes/DM-46599.removal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Regular expressions in collection and dataset type patterns are now deprecated. (Shell-like globs will continue to be supported.)

Materializing dataset queries into temporary tables is now deprecated. (Materializing data ID queries will continue to be supported.)

The `datasetTypes` argument to `Registry.queryCollections` is now deprecated. (This parameter has never had any effect.)

We will soon stop raising `DataIdValueError` exceptions for typos and other bad values in query expressions like `instrument='HsC'` for typos and other bad values in query expressions. Instead, these queries will return an empty iterable of results.

Using HTM and HEALPix spatial dimensions like `htm11` or `healpix10` in data ID constraints passed to queries is now deprecated. The exception is `htm7`, which will continue to work.

The `--no-check` parameter to `butler query-dimension-records` is now deprecated.

The `offset` argument to `limit()` for `Registry.queryDataIds` and `Registry.queryDimensionRecords` result objects is now deprecated.

The `--offset` option for `butler query-data-ids` and `butler-query-datasets` is no longer supported, and will raise on exception if you attempt to use it.

It will soon become mandatory to explicitly provide `--collections` and a dataset type search when calling `butler query-datasets`.

Using `Butler.collections` to get the list of default collections is now deprecated. Use `Butler.collections.defaults` instead.
13 changes: 13 additions & 0 deletions python/lsst/daf/butler/_butler_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from collections.abc import Iterable, Mapping, Sequence, Set
from typing import TYPE_CHECKING, Any, overload

from deprecated.sphinx import deprecated
from pydantic import BaseModel

from ._collection_type import CollectionType
Expand Down Expand Up @@ -83,9 +84,21 @@ def __getitem__(self, index: int) -> str: ...
@overload
def __getitem__(self, index: slice) -> Sequence[str]: ...

@deprecated(
"‘Butler.collections’ should no longer be used to get the list of default collections."
" Use ‘Butler.collections.default’ instead. Will be removed after v28.",
version="v28",
category=FutureWarning,
)
def __getitem__(self, index: int | slice) -> str | Sequence[str]:
return self.defaults[index]

@deprecated(
"‘Butler.collections’ should no longer be used to get the list of default collections."
" Use ‘Butler.collections.default’ instead. Will be removed after v28.",
version="v28",
category=FutureWarning,
)
def __len__(self) -> int:
return len(self.defaults)

Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/cli/cmd/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ def query_data_ids(**kwargs: Any) -> None:
def query_dimension_records(**kwargs: Any) -> None:
"""Query for dimension information."""
if kwargs.pop("no_check") is not None:
click.echo("WARNING: --no-check option now has no effect and is ignored.")
click.echo("WARNING: --no-check option has no effect and will be removed after v28.")
table = script.queryDimensionRecords(**kwargs)
if table:
table.pprint_all()
Expand Down
29 changes: 25 additions & 4 deletions python/lsst/daf/butler/registry/queries/_results.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,15 @@
)

import itertools
import warnings
from abc import abstractmethod
from collections.abc import Iterable, Iterator, Sequence
from contextlib import AbstractContextManager, ExitStack, contextmanager
from types import EllipsisType
from typing import Any, Self

from deprecated.sphinx import deprecated

from ..._dataset_ref import DatasetRef
from ..._dataset_type import DatasetType
from ..._exceptions_legacy import DatasetTypeError
Expand Down Expand Up @@ -527,8 +531,14 @@ def order_by(self, *args: str) -> Self:
self._query = self._query.sorted(clause.terms, defer=True)
return self

def limit(self, limit: int, offset: int | None = 0) -> Self:
if offset is None:
def limit(self, limit: int, offset: int | None | EllipsisType = ...) -> Self:
if offset is not ...:
warnings.warn(
"'offset' parameter should no longer be used. It is not supported by the new query system."
" Will be removed after v28.",
FutureWarning,
)
if offset is None or offset is ...:
offset = 0
self._query = self._query.sliced(offset, offset + limit, defer=True)
return self
Expand Down Expand Up @@ -665,6 +675,11 @@ def byParentDatasetType(self) -> Iterator[ParentDatasetQueryResults]:
yield self

@contextmanager
@deprecated(
"This method should no longer be used. Will be removed after v28.",
version="v28",
category=FutureWarning,
)
def materialize(self) -> Iterator[DatabaseParentDatasetQueryResults]:
# Docstring inherited from DatasetQueryResults.
with self._query.open_context():
Expand Down Expand Up @@ -817,9 +832,15 @@ def order_by(self, *args: str) -> Self:
self._query = self._query.sorted(clause.terms, defer=True)
return self

def limit(self, limit: int, offset: int | None = 0) -> Self:
def limit(self, limit: int, offset: int | None | EllipsisType = ...) -> Self:
# Docstring inherited from base class.
if offset is None:
if offset is not ...:
warnings.warn(
"'offset' parameter should no longer be used. It is not supported by the new query system."
" Will be removed after v28.",
FutureWarning,
)
if offset is None or offset is ...:
offset = 0
self._query = self._query.sliced(offset, offset + limit, defer=True)
return self
Expand Down
6 changes: 6 additions & 0 deletions python/lsst/daf/butler/registry/queries/_sql_query_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

__all__ = ("SqlQueryBackend",)

import warnings
from collections.abc import Iterable, Mapping, Sequence, Set
from contextlib import AbstractContextManager
from typing import TYPE_CHECKING, Any, cast
Expand Down Expand Up @@ -337,6 +338,11 @@ def resolve_governor_constraints(
}
if (constraint_values := constraints.get(dimension_name)) is not None:
if not (constraint_values <= all_values):
warnings.warn(
"DataIdValueError will no longer be raised for invalid governor dimension values."
" Instead, an empty list will be returned. Will be changed after v28.",
FutureWarning,
)
raise DataIdValueError(
f"Unknown values specified for governor dimension {dimension_name}: "
f"{constraint_values - all_values}."
Expand Down
11 changes: 11 additions & 0 deletions python/lsst/daf/butler/registry/queries/_structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
__all__ = ["QuerySummary"] # other classes here are local to subpackage

import dataclasses
import warnings
from collections.abc import Iterable, Mapping, Set
from typing import Any

Expand Down Expand Up @@ -393,6 +394,16 @@ def __init__(
self.order_by = None if order_by is None else OrderByClause.parse_general(order_by, requested)
self.limit = limit
self.columns_required, self.dimensions, self.region = self._compute_columns_required()
for dimension in self.where.data_id.dimensions.names:
if (
dimension.startswith("htm") or dimension.startswith("healpix")
) and not dimension == self.universe.commonSkyPix.name:
warnings.warn(
f"Dimension '{dimension}' should no longer be used in data IDs."
" Use the region 'OVERLAPS' operator in the where clause instead."
" Will be removed after v28.",
FutureWarning,
)

requested: DimensionGroup
"""Dimensions whose primary keys should be included in the result rows of
Expand Down
8 changes: 7 additions & 1 deletion python/lsst/daf/butler/registry/sql_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,12 @@ def queryCollections(
# Right now the datasetTypes argument is completely ignored, but that
# is consistent with its [lack of] guarantees. DM-24939 or a follow-up
# ticket will take care of that.
if datasetType is not None:
warnings.warn(
"The datasetType parameter should no longer be used. It has"
" never had any effect. Will be removed after v28",
FutureWarning,
)
try:
wildcard = CollectionWildcard.from_expression(expression)
except TypeError as exc:
Expand Down Expand Up @@ -2421,7 +2427,7 @@ def queryDatasetAssociations(
if isinstance(datasetType, str):
datasetType = self.getDatasetType(datasetType)
resolved_collections = self.queryCollections(
collections, datasetType, collectionTypes=collectionTypes, flattenChains=flattenChains
collections, collectionTypes=collectionTypes, flattenChains=flattenChains
)
with self._query() as query:
query = query.join_dataset_search(datasetType, resolved_collections)
Expand Down
Loading

0 comments on commit 3abf212

Please sign in to comment.