Skip to content

Commit

Permalink
Remove currently-unusable temporal constraint logic in queries.
Browse files Browse the repository at this point in the history
There's currently no high-level way to populate these arguments to our
lower-level query classes; the only way one can currently do temporal
constraints is to write them out in the 'where' argument, and that
doesn't use any of this.

I've made this a separate commit because I think we may want to revert
it after we've got a public interface (possibly on Butler rather than
Registry) that's designed to support all of the things the daf_relation
query system could now do.  But for now it's just dead weight.
  • Loading branch information
TallJimbo committed Feb 10, 2023
1 parent c8c3dd2 commit 21ed0d5
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 33 deletions.
9 changes: 0 additions & 9 deletions python/lsst/daf/butler/registry/queries/_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,6 @@ def _addWhereClause(self, categorized_columns: ColumnCategorization) -> None:
preferred_engine=self._context.iteration_engine,
transfer=True,
)
if self.summary.timespan is not None:
for element in categorized_columns.filter_timespan_dimension_elements():
self.relation = self.relation.with_rows_satisfying(
self._context.make_timespan_overlap_predicate(
DimensionRecordColumnTag(element, "timespan"), self.summary.timespan
),
preferred_engine=self._context.preferred_engine,
require_preferred_engine=True,
)

def finish(self, joinMissing: bool = True) -> Query:
"""Finish query constructing, returning a new `Query` instance.
Expand Down
27 changes: 3 additions & 24 deletions python/lsst/daf/butler/registry/queries/_structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
NamedValueAbstractSet,
NamedValueSet,
SkyPixDimension,
Timespan,
)

# We're not trying to add typing to the lex/yacc parser code, so MyPy
Expand All @@ -69,7 +68,6 @@ def combine(
bind: Mapping[str, Any] | None = None,
data_id: DataCoordinate | None = None,
region: Region | None = None,
timespan: Timespan | None = None,
defaults: DataCoordinate | None = None,
dataset_type_name: str | None = None,
allow_orphans: bool = False,
Expand All @@ -91,8 +89,6 @@ def combine(
provided, will be set to an empty data ID.
region : `lsst.sphgeom.Region`, optional
A spatial constraint that all rows must overlap.
timespan : `Timespan`, optional
A temporal constraint that all rows must overlap.
defaults : `DataCoordinate`, optional
A data ID containing default for governor dimensions.
dataset_type_name : `str` or `None`, optional
Expand Down Expand Up @@ -126,7 +122,6 @@ def combine(
expression_predicate,
data_id,
region=region,
timespan=timespan,
governor_constraints=governor_constraints,
)

Expand All @@ -145,11 +140,6 @@ def combine(
(`lsst.sphgeom.Region` or `None`).
"""

timespan: Timespan | None
"""A temporal constraint that all result rows must overlap
(`Timespan` or `None`).
"""

governor_constraints: Mapping[str, Set[str]]
"""Restrictions on the values governor dimensions can take in this query,
imposed by the string expression and/or data ID
Expand Down Expand Up @@ -372,7 +362,6 @@ def __init__(
data_id: DataCoordinate | None = None,
expression: str = "",
region: Region | None = None,
timespan: Timespan | None = None,
bind: Mapping[str, Any] | None = None,
defaults: DataCoordinate | None = None,
datasets: Iterable[DatasetType] = (),
Expand All @@ -392,14 +381,13 @@ def __init__(
bind=bind,
data_id=data_id,
region=region,
timespan=timespan,
defaults=defaults,
dataset_type_name=dataset_type_name,
allow_orphans=not check,
)
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.timespan = self._compute_columns_required()
self.columns_required, self.dimensions, self.region = self._compute_columns_required()

requested: DimensionGraph
"""Dimensions whose primary keys should be included in the result rows of
Expand Down Expand Up @@ -439,15 +427,6 @@ def __init__(
which can also come from the data ID passed by the caller.
"""

timespan: Timespan | None
"""Timespan that bounds all query results (`Timespan`).
While `QueryWhereClause.timespan` and the ``timespan`` constructor argument
represent an external timespan given directly by the caller, this
represents the timespan actually used directly as a constraint on the query
results, which can also come from the data ID passed by the caller.
"""

columns_required: Set[ColumnTag]
"""All columns that must be included directly in the query.
Expand All @@ -462,7 +441,7 @@ def universe(self) -> DimensionUniverse:

def _compute_columns_required(
self,
) -> tuple[set[ColumnTag], DimensionGraph, Region | None, Timespan | None]:
) -> tuple[set[ColumnTag], DimensionGraph, Region | None]:
"""Compute the columns that must be provided by the relations joined
into this query in order to obtain the right *set* of result rows in
the right order.
Expand Down Expand Up @@ -517,4 +496,4 @@ def _compute_columns_required(
if missing_common_skypix:
dimensions = dimensions.union(self.universe.commonSkyPix.graph)
tags.update(DimensionKeyColumnTag.generate(dimensions.names))
return (tags, dimensions, region, self.where.timespan)
return (tags, dimensions, region)

0 comments on commit 21ed0d5

Please sign in to comment.