Skip to content

feat: allow callable values in contexts#198

Closed
tlambert03 wants to merge 1 commit intopyapp-kit:mainfrom
tlambert03:main
Closed

feat: allow callable values in contexts#198
tlambert03 wants to merge 1 commit intopyapp-kit:mainfrom
tlambert03:main

Conversation

@tlambert03
Copy link
Copy Markdown
Member

This implements the idea I was mentioning in #196 , wherein a context value is allowed to be a callable (that takes no arguments).

it seems the performance penalty isn't too bad (but of course it would depend on how slow the function is that has to be evaluated to fill in the context:

import timeit

from app_model.expressions import parse_expression

expr = parse_expression("x + 7 < z")


for rn in (1, 2, 3, 4):
    ctx = {str(k): k for k in range(10**rn)}
    ctx["x"] = 3
    ctx["z"] = 10

    N = 100000

    a = timeit.timeit("expr.eval(ctx)", number=N, globals=globals())
    expr.eval(ctx)

    ctx["x"] = lambda: 3
    ctx["z"] = lambda: 10
    b = timeit.timeit("expr.eval_callable_context(ctx)", number=N, globals=globals())

    print(f"with {len(ctx)} variables...")
    print(f"  a is {b/a:.2f} times faster than b")
with 12 variables...
  a is 1.05 times faster than b
with 102 variables...
  a is 1.04 times faster than b
with 1002 variables...
  a is 1.05 times faster than b
with 10002 variables...
  a is 1.06 times faster than b

@tlambert03
Copy link
Copy Markdown
Member Author

tlambert03 commented May 16, 2024

@lucyleeow @DragaDoncila @Czaki ... let me know if you'd like this feature:

it allows this:

In [2]: from app_model.expressions import parse_expression

In [3]: expr = parse_expression("x < 10")

In [4]: expr.eval_callable_context({'x': lambda: 42})
Out[4]: False

essentially, if we made this mode default (or somehow explicitly opt-in), it would allow you to undo the changes made to layer_delegate: https://github.com/napari/napari/blob/4a56237e94480934b1b9f5df5c63949a4059b1f8/napari/_qt/containers/_layer_delegate.py#L325-L328

and instead "permanently" set ctx['valid_spatial_json_clipboard'] = is_valid_spatial_in_clipboard (rather than doing it every time the menu is clicked). And if (and only if) the expression being evaluated needed valid_spatial_json_clipboard would it be evaluated. The downside would be that it would be evaluated for each expression... so if you know that there are a number of expressions that need that value, it will be slower than evaluating it once for all of them.

However, in any case, I still would recommend not calculating is_valid_spatial_in_clipboard every time the layer menu is opened, but rather only once on QClipboard.changed, as mentioned in #196 (comment)

@tlambert03
Copy link
Copy Markdown
Member Author

moving to #199 ... accidentally had this on my main branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant