Conversation
friendlymatthew
changed the title
filter builtin function
crates/monty/src/builtins/filter.rs
Outdated
| let (mut positional, kwargs) = args.into_parts(); | ||
|
|
||
| if !kwargs.is_empty() { | ||
| for (k, v) in kwargs { | ||
| k.drop_with_heap(heap); | ||
| v.drop_with_heap(heap); | ||
| } | ||
|
|
||
| for v in positional { | ||
| v.drop_with_heap(heap); | ||
| } | ||
|
|
||
| return Err(SimpleException::new_msg(ExcType::TypeError, "filter() does not support keyword arguments").into()); | ||
| } | ||
|
|
||
| // Check we have exactly 2 positional arguments | ||
| let positional_len = positional.len(); | ||
| if positional_len != 2 { | ||
| for v in positional { | ||
| v.drop_with_heap(heap); | ||
| } | ||
| return Err(SimpleException::new_msg( | ||
| ExcType::TypeError, | ||
| format!("filter() expected 2 arguments, got {positional_len}"), | ||
| ) | ||
| .into()); | ||
| } | ||
|
|
||
| let function = positional.next().expect("positional must have 2 elements"); | ||
| let iterable = positional.next().expect("positional must have 2 elements"); |
There was a problem hiding this comment.
I think you can use get_one_two_args for this.
There was a problem hiding this comment.
very nice, we can probably update a bunch of other builtin functions like sorted that manually parse args atm
There was a problem hiding this comment.
Yup. Claude can go though and fix them all I guess.
There was a problem hiding this comment.
we also need tests for a non-callable discriminator, like filter(4, [1, 2])
There was a problem hiding this comment.
See crates/monty/test_cases/builtin__filter_not_callable.py
friendlymatthew
force-pushed
the
filter
branch
2 times, most recently
from
fe37fa4 to
7629de6
Compare
samuelcolvin
left a comment
samuelcolvin
left a comment
There was a problem hiding this comment.
Looking good, but I forgot we also need support for use defined functions which might require a bit of logic change, look at list.sort for an example.
I followed monty/crates/monty/src/types/list.rs Lines 922 to 927 in c75f160 |
friendlymatthew
force-pushed
the
filter
branch
from
7629de6 to
f0746a3
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
We need use the new recursion guard, and fix the case of user defined functions. I don't mind on the order. |
2 participants
This PR implements the
filter()builtin function that filters elements from an iterable based on a predicateNote: the implementation returns a list (following the pattern of other iterator-returning builtins like enumerate() and zip() for simplicyt). Currently, only builtin functions are supported as predicates which matches the existing limitation in
sorted()with thekeyparam