From b0d9c3ff83faecaa7709aba336aa65d5c8fdb122 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Thu, 16 May 2024 08:46:26 -0400 Subject: [PATCH 1/8] feat: callable contexts --- src/app_model/expressions/_expressions.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index fdb8d7d0..6351bd65 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -177,6 +177,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: raise RuntimeError("Don't instantiate Expr. Use `Expr.parse`") super().__init__(*args, **kwargs) ast.fix_missing_locations(self) + self._names = set(_iter_names(self)) def eval(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`.""" @@ -186,11 +187,23 @@ def eval(self, context: Mapping[str, object] | None = None) -> T: try: return cast(T, eval(code, {}, context)) except NameError as e: - miss = {k for k in _iter_names(self) if k not in context} + miss = {k for k in self._names if k not in context} raise NameError( f"Names required to eval this expression are missing: {miss}" ) from e + def eval_callable_context(self, context: Mapping[str, object] | None = None) -> T: + """Evaluate this expression with names in `context`.""" + if context is None: + return self.eval() + + context = dict(context) + for name in self._names: + if callable(val := context.get(name)): + context[name] = val() + + return self.eval(context) + @classmethod def parse(cls, expr: str) -> Expr: """Parse string into Expr (classmethod). From 004a9904a3af8f8718634d1126b182efabb100ef Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 17 May 2024 08:06:29 -0400 Subject: [PATCH 2/8] two functions --- src/app_model/expressions/_expressions.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 6351bd65..8a9c0cf5 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -178,8 +178,9 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) ast.fix_missing_locations(self) self._names = set(_iter_names(self)) + self.eval = self.eval_with_callables - def eval(self, context: Mapping[str, object] | None = None) -> T: + def eval_no_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`.""" if context is None: context = {} @@ -192,17 +193,15 @@ def eval(self, context: Mapping[str, object] | None = None) -> T: f"Names required to eval this expression are missing: {miss}" ) from e - def eval_callable_context(self, context: Mapping[str, object] | None = None) -> T: + def eval_with_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`.""" if context is None: - return self.eval() - - context = dict(context) + return self.eval_no_callables(context) + ctx = {} for name in self._names: - if callable(val := context.get(name)): - context[name] = val() - - return self.eval(context) + if name in context: + ctx[name] = val() if callable(val := context[name]) else val + return self.eval_no_callables(ctx) @classmethod def parse(cls, expr: str) -> Expr: From 12cc18934e568277c52de575786b70f2104a6ade Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 17 May 2024 08:16:49 -0400 Subject: [PATCH 3/8] wip --- src/app_model/expressions/_expressions.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 8a9c0cf5..554ecd07 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -201,6 +201,11 @@ def eval_with_callables(self, context: Mapping[str, object] | None = None) -> T: for name in self._names: if name in context: ctx[name] = val() if callable(val := context[name]) else val + else: + miss = {k for k in self._names if k not in context} + raise NameError( + f"Names required to eval this expression are missing: {miss}" + ) return self.eval_no_callables(ctx) @classmethod From 454a9cf955f6861d7bce1a1664b66fe1a360dc0a Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 17 May 2024 08:22:28 -0400 Subject: [PATCH 4/8] reorder --- src/app_model/expressions/_expressions.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 723c4931..26b1af26 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -201,16 +201,6 @@ def eval_no_callables( context = {**context, **ctx_kwargs} return self._eval(context) - def _eval(self, context: Mapping[str, object]) -> T: - """Evaluate this expression using `context`, providing useful error.""" - try: - return eval(self._code, {}, context) # type: ignore - except NameError as e: - miss = {k for k in self._names if k not in context} - raise NameError( - f"Names required to eval this expression are missing: {miss}" - ) from e - def eval_with_callables( self, context: Mapping[str, object] | None = None, **ctx_kwargs: object ) -> T: @@ -233,6 +223,16 @@ def eval_with_callables( ) return self.eval_no_callables(ctx) + def _eval(self, context: Mapping[str, object]) -> T: + """Evaluate this expression using `context`, providing useful error.""" + try: + return eval(self._code, {}, context) # type: ignore + except NameError as e: + miss = {k for k in self._names if k not in context} + raise NameError( + f"Names required to eval this expression are missing: {miss}" + ) from e + @classmethod def parse(cls, expr: str) -> Expr: """Parse string into Expr (classmethod). From 4d2595fce11ec4f7d1f5a32eb512728c775312da Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 17 May 2024 08:26:47 -0400 Subject: [PATCH 5/8] remove ctx_kwargs --- src/app_model/expressions/_expressions.py | 33 +++++++++-------------- tests/test_context/test_expressions.py | 6 ----- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 26b1af26..59253ab1 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -151,9 +151,6 @@ class Expr(ast.AST, Generic[T]): >>> new_expr.eval(dict(v2="hello!", myvar=8)) 'hello!' - you can also use keyword arguments. This is *slightly* slower - >>> new_expr.eval(v2="hello!", myvar=4) - serialize >>> str(new_expr) 'myvar > 5 and v2' @@ -184,33 +181,27 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: self._names = set(_iter_names(self)) self.eval = self.eval_with_callables # type: ignore - def eval( - self, context: Mapping[str, object] | None = None, **ctx_kwargs: object - ) -> T: - """Evaluate this expression with names in `context`.""" + def eval(self, context: Mapping[str, object] | None = None) -> T: + """Evaluate this expression with names in `context`. + + Parameters + ---------- + context : Mapping[str, object] | None + Mapping of names to objects to evaluate the expression with. + """ # will have been replaced in __init__ raise NotImplementedError("This method should have been replaced.") - def eval_no_callables( - self, context: Mapping[str, object] | None = None, **ctx_kwargs: object - ) -> T: + def eval_no_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`.""" if context is None: - context = ctx_kwargs - elif ctx_kwargs: - context = {**context, **ctx_kwargs} + context = {} return self._eval(context) - def eval_with_callables( - self, context: Mapping[str, object] | None = None, **ctx_kwargs: object - ) -> T: + def eval_with_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`, allowing callables.""" if context is None: - context = ctx_kwargs - elif ctx_kwargs: - context = {**context, **ctx_kwargs} - if context is None: - return self._eval(context) + return self._eval({}) ctx = {} for name in self._names: diff --git a/tests/test_context/test_expressions.py b/tests/test_context/test_expressions.py index a62d69fb..71d536df 100644 --- a/tests/test_context/test_expressions.py +++ b/tests/test_context/test_expressions.py @@ -236,12 +236,6 @@ def test_safe_eval(): safe_eval("{1,2,3}") -def test_eval_kwargs(): - expr = parse_expression("a + b") - assert expr.eval(a=1, b=2) == 3 - assert expr.eval({"a": 2}, b=2) == 4 - - @pytest.mark.parametrize("expr", GOOD_EXPRESSIONS) def test_hash(expr): assert isinstance(hash(parse_expression(expr)), int) From 0c4d7b5bb6c43e83fd1830b75a5400f6c34094b7 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 17 May 2024 08:31:55 -0400 Subject: [PATCH 6/8] refactor --- src/app_model/expressions/_expressions.py | 29 +++++++++++------------ 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 59253ab1..bb51ab26 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -196,33 +196,32 @@ def eval_no_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`.""" if context is None: context = {} - return self._eval(context) + try: + return eval(self._code, {}, context) # type: ignore[no-any-return] + except NameError as e: + raise self._missing_names_error(context) from e def eval_with_callables(self, context: Mapping[str, object] | None = None) -> T: """Evaluate this expression with names in `context`, allowing callables.""" if context is None: - return self._eval({}) + return self.eval_no_callables({}) + # build a new context, evaluating any callables + # we only want to evaluate the callables if they are needed, so we + # build a new context with only the names in this expression. ctx = {} for name in self._names: if name in context: ctx[name] = val() if callable(val := context[name]) else val else: - miss = {k for k in self._names if k not in context} - raise NameError( - f"Names required to eval this expression are missing: {miss}" - ) + # early exit if we're missing names + raise self._missing_names_error(context) return self.eval_no_callables(ctx) - def _eval(self, context: Mapping[str, object]) -> T: - """Evaluate this expression using `context`, providing useful error.""" - try: - return eval(self._code, {}, context) # type: ignore - except NameError as e: - miss = {k for k in self._names if k not in context} - raise NameError( - f"Names required to eval this expression are missing: {miss}" - ) from e + def _missing_names_error(self, context: Mapping[str, object]) -> NameError: + """More informative error message when names are missing.""" + miss = {k for k in self._names if k not in context} + return NameError(f"Names required to eval this expression are missing: {miss}") @classmethod def parse(cls, expr: str) -> Expr: From 283fe26852725df5b1c7343d4f089e713b650724 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Fri, 24 May 2024 08:44:13 -0400 Subject: [PATCH 7/8] fix merge --- src/app_model/expressions/_expressions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index 4e5e31a2..e8a66ac4 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -181,6 +181,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: if type(self).__name__ == "Expr": raise RuntimeError("Don't instantiate Expr. Use `Expr.parse`") super().__init__(*args, **kwargs) + self.eval = self.eval_with_callables # type: ignore[method-assign] self._recompile() def _recompile(self) -> None: From a9071f8431500ae8965fb3e7ede94849143ed1ed Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Tue, 28 May 2024 12:35:10 -0400 Subject: [PATCH 8/8] change exception --- src/app_model/expressions/_expressions.py | 6 +++++- tests/test_qt/test_qmenu.py | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/app_model/expressions/_expressions.py b/src/app_model/expressions/_expressions.py index e8a66ac4..7035ad12 100644 --- a/src/app_model/expressions/_expressions.py +++ b/src/app_model/expressions/_expressions.py @@ -229,7 +229,11 @@ def eval_with_callables(self, context: Mapping[str, object] | None = None) -> T: def _missing_names_error(self, context: Mapping[str, object]) -> NameError: """More informative error message when names are missing.""" miss = {k for k in self._names if k not in context} - return NameError(f"Names required to eval this expression are missing: {miss}") + num_keys = len(context) + return NameError( + f"Names required to eval expression '{self}' are missing: {miss}. " + f"Context has {num_keys} keys." + ) @classmethod def parse(cls, expr: str) -> Expr: diff --git a/tests/test_qt/test_qmenu.py b/tests/test_qt/test_qmenu.py index cc28d34c..b6865c23 100644 --- a/tests/test_qt/test_qmenu.py +++ b/tests/test_qt/test_qmenu.py @@ -61,7 +61,9 @@ def test_menu( assert redo_action.isEnabled() # useful error when we forget a required name - with pytest.raises(NameError, match="Names required to eval this expression"): + with pytest.raises( + NameError, match="Names required to eval expression 'allow_undo_redo'" + ): menu.update_from_context({}) menu._disconnect()