Herb-AI / HerbGrammar.jl

Grammars for Herb.jl
https://herb-ai.github.io/
MIT License
0 stars 2 forks source link

Remove `hasdynamicvalue` function calls from rulenode2expr functions #79

Closed To5BG closed 3 months ago

To5BG commented 3 months ago

Remove hasdynamicvalue function calls from rulenode2expr functions because they cause issues with using _val as an evaluation cache for the Probe algorithm.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 46.10%. Comparing base (aca61f9) to head (93bec01).

Files Patch % Lines
src/rulenode_operators.jl 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #79 +/- ## ======================================= Coverage 46.10% 46.10% ======================================= Files 8 8 Lines 462 462 ======================================= Hits 213 213 Misses 249 249 ```

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

sebdumancic commented 3 months ago

This looks fine to me, the original _val feature is not used anywhere. Shall we merge it without @Whebon's approval, as he is probably recovering after his defence? :)

Whebon commented 3 months ago

The original _val is not used anymore, making these checks are redundant. It seems like you have a new use-case for the _val field (otherwise the check returns false). Maybe we should add a docstring to RuleNode to clarify what _val is supposed to do and what format it is expected to be in.