From 01df32d3b8ae0a6c4097b6806de4a5b2f9a10d8f Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Sun, 10 Dec 2023 17:38:39 -0500 Subject: [PATCH] Allow limit=None (sort of) but not offset=None. We mixed up which of these should be allowed to be None in the old interface, but we don't need to repeat the mistake in the new one, though some of the fix will have to wait until new interfaces stop delegating to the old ones. --- python/lsst/daf/butler/_butler.py | 4 ++-- python/lsst/daf/butler/_query_results.py | 18 ++++++++---------- python/lsst/daf/butler/direct_butler.py | 4 ++-- python/lsst/daf/butler/direct_query_results.py | 8 ++++++-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index 655f774fdc..6878bd39eb 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -1382,7 +1382,7 @@ def _query_data_ids( expanded: bool = False, order_by: Iterable[str] | str | None = None, limit: int | None = None, - offset: int | None = None, + offset: int = 0, explain: bool = True, **kwargs: Any, ) -> list[DataCoordinate]: @@ -1569,7 +1569,7 @@ def _query_dimension_records( bind: Mapping[str, Any] | None = None, order_by: Iterable[str] | str | None = None, limit: int | None = None, - offset: int | None = None, + offset: int = 0, explain: bool = True, **kwargs: Any, ) -> list[DimensionRecord]: diff --git a/python/lsst/daf/butler/_query_results.py b/python/lsst/daf/butler/_query_results.py index 7f177f7e98..c1fec87ec4 100644 --- a/python/lsst/daf/butler/_query_results.py +++ b/python/lsst/daf/butler/_query_results.py @@ -389,17 +389,16 @@ def order_by(self, *args: str) -> DataCoordinateQueryResults: raise NotImplementedError() @abstractmethod - def limit(self, limit: int, offset: int | None = 0) -> DataCoordinateQueryResults: + def limit(self, limit: int | None = None, offset: int = 0) -> DataCoordinateQueryResults: """Make the iterator return limited number of records. Parameters ---------- - limit : `int` + limit : `int` or `None`, optional Upper limit on the number of returned records. - offset : `int` or `None`, optional + offset : `int`, optional The number of records to skip before returning at most ``limit`` - records. `None` is interpreted the same as zero for backwards - compatibility. + records. Returns ------- @@ -695,17 +694,16 @@ def order_by(self, *args: str) -> DimensionRecordQueryResults: raise NotImplementedError() @abstractmethod - def limit(self, limit: int, offset: int | None = 0) -> DimensionRecordQueryResults: + def limit(self, limit: int | None = None, offset: int = 0) -> DimensionRecordQueryResults: """Make the iterator return limited number of records. Parameters ---------- - limit : `int` + limit : `int` or `None`, optional Upper limit on the number of returned records. - offset : `int` or `None` + offset : `int`, optional The number of records to skip before returning at most ``limit`` - records. `None` is interpreted the same as zero for backwards - compatibility. + records. Returns ------- diff --git a/python/lsst/daf/butler/direct_butler.py b/python/lsst/daf/butler/direct_butler.py index d9a8b13c08..420d974598 100644 --- a/python/lsst/daf/butler/direct_butler.py +++ b/python/lsst/daf/butler/direct_butler.py @@ -2228,7 +2228,7 @@ def _query_data_ids( expanded: bool = False, order_by: Iterable[str] | str | None = None, limit: int | None = None, - offset: int | None = None, + offset: int = 0, explain: bool = True, **kwargs: Any, ) -> list[DataCoordinate]: @@ -2289,7 +2289,7 @@ def _query_dimension_records( bind: Mapping[str, Any] | None = None, order_by: Iterable[str] | str | None = None, limit: int | None = None, - offset: int | None = None, + offset: int = 0, explain: bool = True, **kwargs: Any, ) -> list[DimensionRecord]: diff --git a/python/lsst/daf/butler/direct_query_results.py b/python/lsst/daf/butler/direct_query_results.py index 7aaae7553c..e4f8c5604f 100644 --- a/python/lsst/daf/butler/direct_query_results.py +++ b/python/lsst/daf/butler/direct_query_results.py @@ -143,8 +143,10 @@ def order_by(self, *args: str) -> DataCoordinateQueryResults: # Docstring inherited. return DirectDataCoordinateQueryResults(self._registry_query_result.order_by(*args)) - def limit(self, limit: int, offset: int | None = 0) -> DataCoordinateQueryResults: + def limit(self, limit: int | None = None, offset: int = 0) -> DataCoordinateQueryResults: # Docstring inherited. + if limit is None: + raise NotImplementedError("Offset without limit is temporarily unsupported.") return DirectDataCoordinateQueryResults(self._registry_query_result.limit(limit, offset)) @@ -289,8 +291,10 @@ def order_by(self, *args: str) -> DimensionRecordQueryResults: # Docstring inherited. return DirectDimensionRecordQueryResults(self._registry_query_result.order_by(*args)) - def limit(self, limit: int, offset: int | None = 0) -> DimensionRecordQueryResults: + def limit(self, limit: int | None = None, offset: int = 0) -> DimensionRecordQueryResults: # Docstring inherited. + if limit is None: + raise NotImplementedError("Offset without limit is temporarily unsupported.") return DirectDimensionRecordQueryResults(self._registry_query_result.limit(limit, offset)) def explain_no_results(self, execute: bool = True) -> Iterable[str]: