Skip to content

Commit 219bbd7

Browse files
svsgooglecopybara-github
authored andcommitted
Remove the with_ancestors argument from DataSliceManagerView.get().
The `with_ancestors` parameter no longer makes sense now that there is a `populate` arg. A better name for it would be `return_populated_root`. But removing it completely is cleaner, since a call of the form `view.get(with_ancestors=True)` is equivalent to `root.get(populate=[view])`. If you don't have the root already, it can easily be obtained using `view.get_root()`. PiperOrigin-RevId: 861678350 Change-Id: Iafaa1e398f36157cdd4690e2751d5d1a88b13c0f
1 parent d354a6b commit 219bbd7

File tree

3 files changed

+20
-43
lines changed

3 files changed

+20
-43
lines changed

py/koladata/ext/persisted_data/colab/Persisted_incremental_DataSlice_basics.ipynb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@
726726
}
727727
],
728728
"source": [
729-
"root.query[:].doc[:].get_data_slice(with_ancestors=True, with_descendants=True)"
729+
"root.get_data_slice(populate_including_descendants=[root.query[:].doc[:]])"
730730
]
731731
},
732732
{

py/koladata/ext/persisted_data/data_slice_manager_view.py

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ def get_schema(self) -> kd.types.SchemaItem:
9898
def get_data_slice(
9999
self,
100100
*,
101-
with_ancestors: bool = False,
102101
with_descendants: bool = False,
103102
populate: Collection[DataSliceManagerView] | None = None,
104103
populate_including_descendants: (
@@ -110,20 +109,13 @@ def get_data_slice(
110109
The view path must be valid, i.e. self.is_view_valid() must be True.
111110
112111
Args:
113-
with_ancestors: If True, then the DataSlice will include the data of all
114-
the ancestors of the view path up to the root. Otherwise, the returned
115-
DataSlice will be start at this view path, and only its data and the
116-
data of its requested descendants (if any) will be accessible in the
117-
result.
118112
with_descendants: If True, then the DataSlice will include the data of all
119113
the descendants of the view path.
120-
populate: Additional views whose DataSlicePaths will be populated. If
121-
these paths are not descendants of this view's path, then you have to
122-
pass with_ancestors=True to see the data in the returned DataSlice.
114+
populate: Additional views whose DataSlicePaths will be populated. Their
115+
paths must be descendants of this view's path for you to see their data
116+
in the returned DataSlice.
123117
populate_including_descendants: Additional views whose DataSlicePaths and
124-
all their descendants will be populated. If these paths are not
125-
descendants of this view's path, then you have to pass
126-
with_ancestors=True to see the data in the returned DataSlice.
118+
all their descendants will be populated.
127119
"""
128120
self._check_path_from_root_is_valid()
129121
populate = {v.get_path_from_root() for v in populate or []}
@@ -138,14 +130,11 @@ def get_data_slice(
138130
populate=populate,
139131
populate_including_descendants=populate_including_descendants,
140132
)
141-
if with_ancestors:
142-
return ds
143133
return self._path_from_root.evaluate(ds)
144134

145135
def get(
146136
self,
147137
*,
148-
with_ancestors: bool = False,
149138
with_descendants: bool = False,
150139
populate: Collection[DataSliceManagerView] | None = None,
151140
populate_including_descendants: (
@@ -157,23 +146,15 @@ def get(
157146
The view path must be valid, i.e. self.is_view_valid() must be True.
158147
159148
Args:
160-
with_ancestors: If True, then the DataSlice will include the data of all
161-
the ancestors of the view path up to the root. Otherwise, the returned
162-
DataSlice will be start at this view path, and only its data and the
163-
data of its requested descendants (if any) will be accessible in the
164-
result.
165149
with_descendants: If True, then the DataSlice will include the data of all
166150
the descendants of the view path.
167-
populate: Additional views whose DataSlicePaths will be populated. If
168-
these paths are not descendants of this view's path, then you have to
169-
pass with_ancestors=True to see the data in the returned DataSlice.
151+
populate: Additional views whose DataSlicePaths will be populated. Their
152+
paths must be descendants of this view's path for you to see their data
153+
in the returned DataSlice.
170154
populate_including_descendants: Additional views whose DataSlicePaths and
171-
all their descendants will be populated. If these paths are not
172-
descendants of this view's path, then you have to pass
173-
with_ancestors=True to see the data in the returned DataSlice.
155+
all their descendants will be populated.
174156
"""
175157
return self.get_data_slice(
176-
with_ancestors=with_ancestors,
177158
with_descendants=with_descendants,
178159
populate=populate,
179160
populate_including_descendants=populate_including_descendants,

py/koladata/ext/persisted_data/data_slice_manager_view_test.py

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def test_typical_usage(self):
7171
)
7272
restricted_query_schema = kd.named_schema('query', query_id=kd.INT32)
7373
kd.testing.assert_equivalent(
74-
queries.query_id.get(with_ancestors=True),
74+
root.get(populate=[queries.query_id]),
7575
kd.new(
7676
query=kd.list([
7777
restricted_query_schema.new(query_id=0),
@@ -1800,19 +1800,17 @@ def test_get_with_populate_arguments(self):
18001800

18011801
kd.testing.assert_equivalent(
18021802
doc_view.doc_id.get(
1803-
# By default, with_ancestors=False. Since the query text is not a
1804-
# descendant of the doc_id, only the doc_id will be visible in the
1805-
# result.
1803+
# Since the query text is not a descendant of the doc_id, only the
1804+
# doc_id will be visible in the result.
18061805
populate=[query_view.text],
18071806
),
18081807
query_list[:].doc[:].doc_id,
18091808
)
18101809
kd.testing.assert_equivalent(
1811-
doc_view.doc_id.get(
1810+
root_view.get(
18121811
# This time we request the result to start from the root, so the
18131812
# query text will be visible in the result.
1814-
with_ancestors=True,
1815-
populate=[query_view.text],
1813+
populate=[query_view.text, doc_view.doc_id],
18161814
),
18171815
kd.new(
18181816
query=kd.list([
@@ -1865,22 +1863,20 @@ def test_get_with_populate_arguments(self):
18651863
ids_equality=True,
18661864
)
18671865
kd.testing.assert_equivalent(
1868-
doc_view.get(
1869-
with_ancestors=True, populate_including_descendants=[query_view]
1866+
root_view.get(
1867+
populate=[doc_view], populate_including_descendants=[query_view]
18701868
),
18711869
root_view.get(with_descendants=True),
18721870
ids_equality=True,
18731871
)
18741872
kd.testing.assert_equivalent(
1875-
query_view.query_id.get(
1876-
with_ancestors=True,
1877-
populate=[query_view.text],
1873+
root_view.get(
1874+
populate=[query_view.text, query_view.query_id],
18781875
populate_including_descendants=[doc_view],
18791876
),
1880-
doc_view.get(
1881-
with_ancestors=True,
1882-
with_descendants=True,
1877+
root_view.get(
18831878
populate=[query_view.text, query_view.query_id],
1879+
populate_including_descendants=[doc_view],
18841880
),
18851881
ids_equality=True,
18861882
)

0 commit comments

Comments
 (0)