Add FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetX, SumX, ProductX#3357
Add FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetX, SumX, ProductX#3357fingolfin wants to merge 8 commits intogap-system:masterfrom
FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetX, SumX, ProductX#3357Conversation
|
I think this looks very reasonable. For the shortcutting, I'm tempted to suggest an optional single argument which is a object which is compared using IsIdenticalObj. My reasoning is:
|
|
@ChrisJefferson I like that idea! We could simply pass |
|
I like this project. |
Codecov Report
@@ Coverage Diff @@
## master #3357 +/- ##
===========================================
+ Coverage 82.18% 93.39% +11.21%
===========================================
Files 678 716 +38
Lines 287772 812207 +524435
===========================================
+ Hits 236500 758590 +522090
- Misses 51272 53617 +2345
|
711396f to
edb1c8e
Compare
Turn FoldLeft into an operation with early methods for built-in lists, make it skip holes consistently, and tighten FoldLeftX and the X-style boolean predicates to match the surrounding collection API semantics. Complete the manual entries for FoldLeft, FoldLeftX, and the X-style helper functions, and extend the collection tests to cover hole skipping, FoldLeftX validation, short-circuiting, and boolean result checks. AI assistance was provided by Codex for reviewing the branch, implementing the changes, updating documentation, and preparing this commit message. Co-authored-by: Codex <codex@openai.com>
|
Almost exactly 7 years after opening this PR, finally it is ready for review. Thanks to Codex for the final commit that finished the documentation, discovered a few edge cases in input validation, and added more tests. I hope this can get into GAP 4.16 :-) |
FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetX
FoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetXFoldLeft, FoldLeftX; and ForAllX, ForAnyX, FilteredX, NumberX, PerformX to complement ListX, SetX, SumX, ProductX
ThomasBreuer
left a comment
There was a problem hiding this comment.
Nice.
Besides my few comments, perhaps a cross-reference from ListX to FoldLeftX could be added.
First I thought that manual examples would be nice but also SetX and SumX have no examples, they just refer to ListX.
Tighten the manual wording around FoldLeftX and the related X-functions after review feedback. Clarify how functions in gens receive their arguments, fix FoldLeft references now that it is an operation, and add a cross-reference from ListX to FoldLeftX. Also refresh a few examples to use the shorter function syntax where it improves readability, without changing behavior or broadening the scope of the PR. AI assistance was provided by Codex for reviewing the feedback, updating the documentation, and preparing this commit message. Co-authored-by: Codex <codex@openai.com>
Replace the placeholder TODO comments in the FoldLeftX kernel helper and entry point with brief descriptions of the traversal, tuple workspace, and abort-value short-circuiting logic. AI assistance was provided by Codex for identifying the remaining TODO markers, updating the comments, and preparing this commit message. Co-authored-by: Codex <codex@openai.com>
|
@ThomasBreuer I hope I've addressed all your points. |
|
Yes (and more). |
|
Nice job! ❤️ |
This PR adds a new operation
FoldLeftand a more general helperFoldLeftX,then uses
FoldLeftXto simplify the implementation of the existingListX,SumX, andProductXfunctions and to add several furtherX-style functions:ForAllX,ForAnyX,FilteredX,NumberX, andPerformX.FoldLeftis implemented as an operation with early methods for built-inlists and regular methods for general collections. In particular, lists with
holes are handled consistently by iterating past holes, just as in ordinary
for-loops.FoldLeftXtakes a list of generators and filters describing nested loops andconditions, together with an accumulator update function and an initial value.
It also supports an optional stopping value for efficient short-circuiting,
which is used by
ForAllXandForAnyX.One idea for a future follow-up would be to broaden the scope and allow
ListXand related functions to accept iterators directly. But this PRhas already been here since 2019 (and the underlying code even longer),
so I'd rather not tackle this now.
AI disclosure: I used Codex for reviewing the branch, implementing the final
polish, extending tests, and completing the documentation.