Implement ArrayVar operations: filter, reduce, flat_map, foreach -> map#6701
Implement ArrayVar operations: filter, reduce, flat_map, foreach -> map#6701masenf wants to merge 1 commit into
Conversation
implements Array -> new Array ops as var operations on ArrayVar renames `.foreach` to `.map` for consistency with js and python
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThis PR implements Confidence Score: 4/5Safe to merge if the project's minimum Python version is 3.9+; verify before landing to avoid a runtime crash in The implementation is clean and comprehensive with strong test coverage. The primary concern is the runtime subscript of packages/reflex-base/src/reflex_base/vars/sequence.py — the Important Files Changed
Reviews (1): Last reviewed commit: "Implement ArrayVar operations: filter, r..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if TYPE_CHECKING: | ||
|
|
||
| @deprecated("Use `ArrayVar.map` instead.") | ||
| def foreach(self, fn: Any) -> Var[list[Any]]: | ||
| """Apply a function to each element of the array (deprecated, use map). | ||
|
|
||
| Args: | ||
| fn: The function to apply, taking at most one argument. | ||
|
|
||
| Returns: | ||
| The array after applying the function. | ||
| """ | ||
| ... | ||
|
|
||
| first_arg_type = self[number_var]._var_type | ||
| else: | ||
|
|
||
| arg_name = get_unique_variable_name() | ||
| def foreach(self, fn: Any): | ||
| """Apply a function to each element of the array (deprecated, use map). | ||
|
|
||
| # get first argument type | ||
| first_arg = Var( | ||
| _js_expr=arg_name, | ||
| _var_type=first_arg_type, | ||
| ).guess_type() | ||
| Args: | ||
| fn: The function to apply, taking at most one argument. | ||
|
|
||
| function_var = ArgsFunctionOperation.create( | ||
| (arg_name,), | ||
| Var.create(fn(first_arg)), | ||
| Returns: | ||
| The array after applying the function. | ||
| """ | ||
| console.deprecate( | ||
| feature_name="ArrayVar.foreach", | ||
| reason="Use ArrayVar.map instead.", | ||
| deprecation_version="0.9.7", | ||
| removal_version="1.0", | ||
| ) | ||
| return self.map(fn) |
There was a problem hiding this comment.
if TYPE_CHECKING split
Defining the same method in two branches of an if TYPE_CHECKING: block inside a class body is valid Python but unusual enough to be a maintenance trap. The runtime branch already calls console.deprecate, so the only thing the TYPE_CHECKING stub adds is the static-analysis decorator. A single method with typing_extensions.deprecated applied unconditionally (it is a no-op at runtime unless __deprecated__ is inspected) would be simpler and equally effective.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| function_var = ArgsFunctionOperation.create( | ||
| (accumulator._js_expr, element._js_expr), | ||
| return_expr, | ||
| _var_type=Callable[ | ||
| [accumulator_type, element._var_type], return_expr._var_type | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Callable subscript is a Python 3.9+ feature when imported from collections.abc
Callable[[accumulator_type, element._var_type], return_expr._var_type] uses collections.abc.Callable (imported at the top of the file). Subscripting collections.abc.Callable directly requires Python 3.9+; on 3.8 it raises TypeError: 'ABCMeta' object is not subscriptable. If the project still targets Python 3.8, this line would crash whenever reduce is called. Using typing.Callable instead would be safe across all supported versions.
| function_var = ArgsFunctionOperation.create( | |
| (accumulator._js_expr, element._js_expr), | |
| return_expr, | |
| _var_type=Callable[ | |
| [accumulator_type, element._var_type], return_expr._var_type | |
| ], | |
| ) | |
| import typing as _typing | |
| function_var = ArgsFunctionOperation.create( | |
| (accumulator._js_expr, element._js_expr), | |
| return_expr, | |
| _var_type=_typing.Callable[ | |
| [accumulator_type, element._var_type], return_expr._var_type | |
| ], | |
| ) |
| return_type = callable_args[-1] if callable_args else Any | ||
| if initial is not None: | ||
| return var_operation_return( | ||
| js_expression=f"{array}.reduce({function}, {initial})", | ||
| var_type=unionize(return_type, initial._var_type), | ||
| ) | ||
| param_types = callable_args[0] if callable_args else None | ||
| element_type = ( | ||
| param_types[1] | ||
| if isinstance(param_types, list) and len(param_types) == 2 | ||
| else Any | ||
| ) | ||
| return var_operation_return( | ||
| js_expression=f"{array}.reduce({function})", | ||
| var_type=unionize(return_type, element_type), | ||
| ) | ||
|
|
||
|
|
||
| _PY_FLAT_MAP_IMPORT: ImportDict = { | ||
| f"$/{Dirs.STATE_PATH}": [ImportVar(tag="pyFlatMap")], |
There was a problem hiding this comment.
get_args shape differs between Python versions, silently falling back to Any
typing.get_args(Callable[[A, B], R]) returns ([A, B], R) on Python 3.9+ but (A, B, R) on Python 3.8. The guard isinstance(param_types, list) only matches the 3.9+ shape; on 3.8, element_type silently becomes Any, so the inferred return type of reduce without an initial value is Any rather than the correct union. If Python 3.8 is still supported this is a silent type-quality regression; if it isn't, a comment clarifying the assumption would help.
1 participant
implements Array -> new Array ops as var operations on ArrayVar
renames
.foreachto.mapfor consistency with js and python