-
Notifications
You must be signed in to change notification settings - Fork 4
✨ feat(lang): add 8 useful utility functions and macros #1043
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.
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
Conversation
Add three new utility macros to builtin.mq: - when: Executes expression only if condition is true - default_to: Returns default value if input is None or empty - compact_map: Maps over array and removes None values from result Also adds comprehensive tests for all three macros.
Add new utility functions and macros to builtin.mq: - reject: Filters out elements matching a condition (opposite of filter) - partition: Splits array into [matching, not_matching] based on condition - get_or: Safely gets dict value with default fallback - times: Executes expression n times and returns array of results - between: Checks if value is between min and max (inclusive) - not_empty: Checks if value is not empty (opposite of is_empty) - sum_by: Sums array elements after applying transformation function - index_by: Creates dictionary indexed by key extracted from elements All functions include comprehensive tests covering edge cases and typical usage patterns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds 8 new utility functions and macros to the mq language's builtin library to enhance its functional programming capabilities. The additions provide convenient wrappers for common operations like filtering, partitioning, safe dictionary access, and array transformations.
Key Changes:
- Added filtering utilities:
reject,partition,compact_map - Added safe access patterns:
get_or,default_to - Added utility macros:
times,between,not_empty,when - Added aggregation helpers:
sum_by,index_by
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/mq-lang/builtin.mq | Implements 8 new utility functions and macros including reject, partition, get_or, times, between, not_empty, sum_by, and index_by |
| crates/mq-lang/builtin_tests.mq | Adds comprehensive test coverage for all 8 new utility functions, verifying edge cases and expected behavior |
crates/mq-lang/builtin.mq
Outdated
| quote do | ||
| let __ib_arr = unquote(ib_array) | ||
| | fold(__ib_arr, dict(), fn(acc, item): | ||
| set(acc, to_string(unquote(ib_key_expr)), item); |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The index_by macro converts keys to strings with to_string, which means numeric keys will be stored as strings. This could lead to unexpected behavior when accessing the resulting dictionary. The test on line 610-611 uses string keys "1" and "2" to access the dict, confirming this string conversion happens, but users might expect numeric keys to remain numeric. Consider documenting this behavior or providing a way to preserve key types.
CodSpeed Performance ReportMerging #1043 will degrade performance by 12.59%Comparing Summary
Benchmarks breakdown
Footnotes |
Remove the when macro and its associated test as it's redundant with the existing if expression syntax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| | assert_eq(result5, false) | ||
| end | ||
|
|
||
| | def test_sum_by(): |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test_sum_by function lacks test coverage for non-numeric results or edge cases where the transformation function might return non-addable values. Consider adding a test case that verifies behavior when the transformation produces unexpected types.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
…s to functions Converts three macros to simpler function implementations: - default_to: returns default value if input is None or empty - get_or: safely gets dict value with default fallback - between: checks if value is within min/max range (inclusive)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| | def test_sum_by(): | ||
| let result1 = sum_by([1, 2, 3, 4], fn(x): x;) | ||
| | assert_eq(result1, 10) | ||
|
|
||
| | let result2 = sum_by([1, 2, 3], fn(x): x * 2;) | ||
| | assert_eq(result2, 12) | ||
|
|
||
| | let result3 = sum_by([], fn(x): x;) | ||
| | assert_eq(result3, 0) | ||
|
|
||
| | let result4 = sum_by([5, 10, 15], fn(x): x / 5;) |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test cases for sum_by only cover numeric values. Consider adding test coverage for edge cases such as attempting to sum non-numeric values or handling division by zero within the transformation function.
| | def test_index_by(): | ||
| let arr = [dict([["id", 1], ["name", "Alice"]]), dict([["id", 2], ["name", "Bob"]])] | ||
| | let result1 = index_by(arr, fn(item): item["id"];) | ||
| | assert_eq(get(result1, "1")["name"], "Alice") | ||
| | assert_eq(get(result1, "2")["name"], "Bob") | ||
|
|
||
| | let result2 = index_by([], fn(item): item["id"];) | ||
| | assert_eq(result2, dict()) |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test_index_by function lacks test coverage for key collision scenarios. Given the implementation uses existing + item when keys collide, there should be a test case verifying this behavior when multiple items produce the same key.
Add new utility functions and macros to builtin.mq: