google / cel-go

Fast, portable, non-Turing complete expression evaluation with gradual typing (Go)
https://cel.dev
Apache License 2.0
2.19k stars 218 forks source link

Add list flatten() #980

Closed fabriziosestito closed 3 weeks ago

fabriziosestito commented 2 months ago

Adds list flatten, fixing #352

fabriziosestito commented 2 months ago

@seirl I took inspiration from Elixir's List.flatten. Also, Ruby's Array.flatten is recursive by default, but it accepts an optional level argument. We could consider this behaviour, as it is more flexible.

However, I don't have a strong opinion and I am ok with what you proposed.

seirl commented 2 months ago

Interesting. FWIW:

The reason I'm mentioning this is because flatten could also be useful to flatten a list(optional(T)) into a list(T), but the semantics are more complex if you have a recursive flatten.

fabriziosestito commented 2 months ago

@seirl Alright, I will change this to single-level flattening.

fabriziosestito commented 1 month ago

@seirl I've followed your suggestion, now flatten flattens to a single level, while flattenDeep flattens recursively. I've preferred the deep naming as I find it clearer for the user and it is also used by lodash.

TristonianJones commented 1 month ago

/gcbrun

TristonianJones commented 1 month ago

/gcbrun

TristonianJones commented 4 weeks ago

/gcbrun

fabriziosestito commented 3 weeks ago

@TristonianJones, thank you for merging. I'm currently on vacation with limited laptop access. I'll handle the cost estimation if the task is still open when I return. Regarding the function signature, shouldn't it be something like flatten<T>(list(T | list(T))) -> list(T)?

seirl commented 3 weeks ago

@fabriziosestito the one-level .flatten() should only flatten List[List[T]], so [1, [2, 3]].flatten() should be an error.

If you use only strongly-typed CEL, all your lists need to be homogeneous, so [1, [2, 3]] doesn't make sense as a value.

TristonianJones commented 3 weeks ago

@seirl I took care of the signature change in a different PR. The test case will pass checking because the mixed type list is list(dyn)