-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add aiken/collection/pairs.{merge}
#93
Open
ariady-putra
wants to merge
2
commits into
aiken-lang:main
Choose a base branch
from
ariady-putra:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+102
−1
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
//// > and thus allow for duplicate keys. This is reflected in the functions used | ||
//// > to interact with them. | ||
|
||
use aiken/builtin | ||
use aiken/primitive/bytearray | ||
|
||
// ## Inspecting | ||
|
@@ -572,6 +573,106 @@ test map_2() { | |
map(fixture, with: fn(_, v) { v + 1 }) == [Pair("a", 2), Pair("b", 3)] | ||
} | ||
|
||
/// Merge a value into the `Pairs` at a given key. If the key already exists, | ||
/// its value is merged by the merging function. | ||
/// | ||
/// ```aiken | ||
/// use aiken/builtin | ||
/// use aiken/primitive/bytearray | ||
/// | ||
/// let compare_un_b_data = fn(l, r) { | ||
/// bytearray.compare(l |> builtin.un_b_data, r |> builtin.un_b_data) | ||
/// } | ||
/// | ||
/// let result = | ||
/// [] | ||
/// |> pairs.merge(key: "foo" |> builtin.b_data, value: 1, compare: compare_un_b_data, merging: builtin.add_integer) | ||
/// |> pairs.merge(key: "bar" |> builtin.b_data, value: 2, compare: compare_un_b_data, merging: builtin.add_integer) | ||
/// |> pairs.merge(key: "foo" |> builtin.b_data, value: 3, compare: compare_un_b_data, merging: builtin.add_integer) | ||
/// | ||
/// result == [Pair("bar" |> builtin.b_data, 2), Pair("foo" |> builtin.b_data, 4)] | ||
/// ``` | ||
/// | ||
/// Notice that the key is of type `Data`. The function works with any key type, not just ByteArray. | ||
/// Unlike `dict.from_pairs() |> dict.union_with() |> dict.to_pairs()` | ||
pub fn merge( | ||
self: Pairs<key, value>, | ||
key k: key, | ||
value v: value, | ||
compare: fn(key, key) -> Ordering, | ||
merging: fn(value, value) -> value, | ||
) -> Pairs<key, value> { | ||
when self is { | ||
[] -> | ||
[Pair(k, v)] | ||
|
||
[Pair(k2, v2), ..rest] -> | ||
when compare(k, k2) is { | ||
Less -> | ||
[Pair(k, v), ..self] | ||
Comment on lines
+611
to
+612
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is unfortunately not possible in this context. |
||
|
||
Equal -> | ||
[Pair(k, merging(v, v2)), ..rest] | ||
|
||
Greater -> | ||
[Pair(k2, v2), ..merge(rest, k, v, compare, merging)] | ||
} | ||
} | ||
} | ||
|
||
test merge_1() { | ||
let compare_un_b_data = | ||
fn(l, r) { | ||
bytearray.compare(l |> builtin.un_b_data, r |> builtin.un_b_data) | ||
} | ||
|
||
let m = | ||
[] | ||
|> merge("foo" |> builtin.b_data, 42, compare_un_b_data, builtin.add_integer) | ||
|> merge("foo" |> builtin.b_data, 14, compare_un_b_data, builtin.add_integer) | ||
|
||
m == [Pair("foo" |> builtin.b_data, 56)] | ||
} | ||
|
||
test merge_2() { | ||
let compare_un_b_data = | ||
fn(l, r) { | ||
bytearray.compare(l |> builtin.un_b_data, r |> builtin.un_b_data) | ||
} | ||
|
||
let m = | ||
[] | ||
|> merge("foo" |> builtin.b_data, 42, compare_un_b_data, builtin.add_integer) | ||
|> merge("bar" |> builtin.b_data, 14, compare_un_b_data, builtin.add_integer) | ||
|> merge( | ||
"baz" |> builtin.b_data, | ||
1337, | ||
compare_un_b_data, | ||
builtin.add_integer, | ||
) | ||
|
||
m == [ | ||
Pair("bar" |> builtin.b_data, 14), | ||
Pair("baz" |> builtin.b_data, 1337), | ||
Pair("foo" |> builtin.b_data, 42), | ||
] | ||
} | ||
|
||
test merge_3() { | ||
let compare_un_b_data = | ||
fn(l, r) { | ||
bytearray.compare(l |> builtin.un_b_data, r |> builtin.un_b_data) | ||
} | ||
|
||
let result = | ||
[] | ||
|> merge("foo" |> builtin.b_data, 1, compare_un_b_data, builtin.add_integer) | ||
|> merge("bar" |> builtin.b_data, 2, compare_un_b_data, builtin.add_integer) | ||
|> merge("foo" |> builtin.b_data, 3, compare_un_b_data, builtin.add_integer) | ||
|
||
result == [Pair("bar" |> builtin.b_data, 2), Pair("foo" |> builtin.b_data, 4)] | ||
} | ||
|
||
/// Insert a value in the `Pairs` at a given key. If the key already exists, | ||
/// its value is replaced. | ||
/// | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Note that since
Pairs
do not necessarily enforce uniqueness, this function becomes ambiguous:This explains the choice of naming across this module like
delete_first
,delete_last
,delete_all
, to clearly indicates this behavior upfront. So in a similar spirit, I rather have amerge_first
,merge_last
andmerge_all
group.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.
Hey @KtorZ , the "need" of this
merge
function came from here:https://github.com/ariady-putra/tx_util/blob/1399949737ef1ea8c4d65b3cff7d601c25bcd725/lib/tx_util/builder/txn.ak#L484-L495
and even the merging logic itself took inspiration from
repsert_by_ascending_key
, but instead of replacing, we can provide our logics.So the idea is to enable people to do something like:
What's your opinion on this? Or alternative? How about
merge_by_ascending_key
, since the logics are the same except for the replacing part?PS. Idk if that makes sense to do that, but idk could be handy for certain scenarios
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.
or, add a replacing fn param at
repsert_by_ascending_key
?