feat: Query Builder API implemented with Count#584
feat: Query Builder API implemented with Count#584sriramk03 wants to merge 2 commits intoOpenMined:mainfrom
Conversation
dvadym
left a comment
There was a problem hiding this comment.
Thanks! Some comments below
| @@ -0,0 +1,521 @@ | |||
| # Copyright 2023 OpenMined. | |||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| """Advanced query builder API.""" |
There was a problem hiding this comment.
Let's drop advanced, just query_builder.py
| from pipeline_dp import pipeline_backend | ||
|
|
||
|
|
||
| class GroupsBalance(enum.Enum): |
There was a problem hiding this comment.
Let's drop GroupsBalance (it's in supported in Python)
| public_groups: Optional[Sequence[Any]] | ||
| groups_balance: Optional[GroupsBalance] | ||
|
|
||
| class Builder: |
There was a problem hiding this comment.
No need in builder for dataclass, it's more Java style to have builders (because in Java on named arguments, no default values for argument), in Python constructors are used
|
|
||
|
|
||
| @dataclasses.dataclass | ||
| class GaussianCountSpec(CountSpec): |
There was a problem hiding this comment.
We don't need separately Gaussian/Laplace, just CountSpec. See Kotlin file for the texample
| # max_contributions_per_partition is implicitly 1 | ||
|
|
||
|
|
||
| class Budget(abc.ABC): |
There was a problem hiding this comment.
Let's have only 1 dataclass (which contains epsilon and delta, with delta = 0 as a default values)
|
|
||
| def run(self, test_mode: bool = False): | ||
| """Runs the DP query.""" | ||
| backend = pipeline_backend.LocalBackend() |
There was a problem hiding this comment.
Replace the implementation of run to raise NotImplemented(), the purpose of this PR is skeleton, not the implementation. "1 purpose for 1 PR" ))
| contribution_bounding_level: Optional[ContributionBoundingLevel] | ||
| budget: EpsDeltaBudget | ||
|
|
||
| class Builder: |
|
|
||
|
|
||
| @dataclasses.dataclass | ||
| class OptimalGroupSelectionGroupBySpec(GroupBySpec): |
There was a problem hiding this comment.
we don't need this class, from GroupSpec
Description
The design of this new API is based on the principles of the PlumeKotlin public API, which uses a guided, multi-builder pattern to ensure queries are constructed in a valid and logical sequence. This enforcement of the FROM -> GROUP BY -> AGGREGATE order at the API level makes the query construction process more robust and intuitive.
This initial pull request establishes the foundation of the new API, implementing the core builder structure with support for count aggregations.
Implementation Details
The new Advanced Query Builder is implemented through a sequence of specialized builder classes:
How has this been tested?
Checklist