Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] Add a zipped range utility #36367

Closed
bkietz opened this issue Jun 28, 2023 · 0 comments · Fixed by #36393
Closed

[C++] Add a zipped range utility #36367

bkietz opened this issue Jun 28, 2023 · 0 comments · Fixed by #36393
Assignees
@bkietz
Labels
Component: C++ Type: enhancement
Milestone
13.0.0

Comments

@bkietz
Copy link
Member

bkietz commented Jun 28, 2023

Describe the enhancement requested

C++23 adds an analog of Python's zip builtin, enabling parallel iteration of multiple ranges. This doesn't depend on any language features we can't use and would be quite handy for C++ in arrow

Component(s)

C++
bkietz added a commit to bkietz/arrow that referenced this issue Jun 29, 2023
@bkietz
apacheGH-36367: [C++] Add a zipped range utility
41ef18a
bkietz added a commit to bkietz/arrow that referenced this issue Jun 29, 2023
@bkietz
apacheGH-36367: [C++] Add a zipped range utility
571783f
@github-actions github-actions bot mentioned this issue Jun 29, 2023
Merged
pitrou pushed a commit to bkietz/arrow that referenced this issue Jul 4, 2023
@bkietz @pitrou
apacheGH-36367: [C++] Add a zipped range utility
8889373
pitrou pushed a commit to bkietz/arrow that referenced this issue Jul 4, 2023
@bkietz @pitrou
apacheGH-36367: [C++] Add a zipped range utility
ca70791
pitrou pushed a commit to bkietz/arrow that referenced this issue Jul 6, 2023
@bkietz @pitrou
apacheGH-36367: [C++] Add a zipped range utility
1eb100d
pitrou pushed a commit that referenced this issue Jul 6, 2023
@bkietz
GH-36367: [C++] Add a zipped range utility (#36393)
7a5f57f 
### Rationale for this change

We write a lot of loops over parallel ranges. This function is a proven pattern improving code clarity in other languages

### What changes are included in this PR?

A zip utility is added which simplifies writing loops over parallel ranges.

```diff
@@ -1118,9 +1118,8 @@ Status GetFieldsFromArray(const RjArray& json_fields, FieldPosition parent_pos,
                           DictionaryMemo* dictionary_memo,
                           std::vector<std::shared_ptr<Field>>* fields) {
   fields->resize(json_fields.Size());
-  for (rj::SizeType i = 0; i < json_fields.Size(); ++i) {
-    RETURN_NOT_OK(GetField(json_fields[i], parent_pos.child(static_cast<int>(i)),
-                           dictionary_memo, &(*fields)[i]));
+  for (auto [json_field, field, i] : Zip(json_fields, *fields, Enumerate<int>)) {
+    RETURN_NOT_OK(GetField(json_field, parent_pos.child(i), dictionary_memo, &field));
   }
   return Status::OK();
 }
```

Some(most) of the loops in json_internal.cc are rewritten to showcase usage.

### Are these changes tested?

Yes, in `range_test.cc`.

### Are there any user-facing changes?

No, this is an internal utility.

* Closes: #36367

Authored-by: Benjamin Kietzman <bengilgit@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou pitrou added this to the 13.0.0 milestone Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bkietz bkietz

Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-36367: [C++] Add a zipped range utility bkietz/arrow
2 participants
@bkietz @pitrou