search

biolab / orange-canvas-core

Orange Canvas core workflow editor
GNU General Public License v3.0
36 stars 62 forks source link

Evaluate values of dynamic signals before type checking #280

Closed janezd closed 1 year ago

janezd commented 1 year ago

As proposed by @ales-erjavec, this fixes https://github.com/biolab/orange3/issues/6583.

Lazy signals are evaluating before checking their types. If the signal is of the correct type, it would eventually be evaluated anyway, so we don't lose anything.

If lazy signals had type hints, we could check the type before evaluation and save some memory when signals don't match. But this is an error condition and I won't lose sleep over it. Also, type hints would in most cases need to be added "manually" as an additional argument to LazyValue. In most widgets we would forger to do it, so signal manager would have to fall back to evaluation anyway.

janezd commented 1 year ago

@PrimozGodec, can you test this, and merge if it works?

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.02% :warning:

Comparison is base (e85b33f) 75.48% compared to head (3b84ad9) 75.47%. Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #280 +/- ## ========================================== - Coverage 75.48% 75.47% -0.02% ========================================== Files 99 99 Lines 21031 21033 +2 ========================================== - Hits 15876 15875 -1 - Misses 5155 5158 +3 ``` | [Files Changed](https://app.codecov.io/gh/biolab/orange-canvas-core/pull/280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biolab) | Coverage Δ | | |---|---|---| | [orangecanvas/scheme/signalmanager.py](https://app.codecov.io/gh/biolab/orange-canvas-core/pull/280?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biolab#diff-b3JhbmdlY2FudmFzL3NjaGVtZS9zaWduYWxtYW5hZ2VyLnB5) | `88.24% <0.00%> (-0.34%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/biolab/orange-canvas-core/pull/280/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biolab)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

PrimozGodec commented 1 year ago

Yep, it solves the problem. Thank you @janezd and @ales-erjavec