Herb-AI / HerbGrammar.jl

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

Remove line numbers in PCSG construction #84

Closed ReubenJ closed 3 months ago

ReubenJ commented 3 months ago

While adding a test for this case of constructing a probabilistic CSG

https://github.com/Herb-AI/HerbGrammar.jl/blob/d3ff2322448651acac4625adfbafe7c22fcae3fa/src/csg/probabilistic_csg.jl#L71-L74

I ran into a BoundsError. It was only hit when using the @pcsgrammar macro within the @test_logs macro. This was because the @test_logs macro removes the line numbers of any input expression, while our implementation for @pcsgrammar relied on the existence of the line numbers.

Error

```julia HerbGrammar.jl: Error During Test at HerbGrammar.jl/test/runtests.jl:5 Got exception outside of a @test LoadError: BoundsError: attempt to access 1-element Vector{Any} at index [2] Stacktrace: [1] getindex(A::Vector{Any}, i1::Int64) @ Base ./essentials.jl:13 [2] parse_probabilistic_rule(e::Expr) @ HerbGrammar HerbGrammar.jl/src/csg/probabilistic_csg.jl:72 [3] expr2pcsgrammar(ex::Expr) @ HerbGrammar HerbGrammar.jl/src/csg/probabilistic_csg.jl:26 [4] var"@pcsgrammar"(__source__::LineNumberNode, __module__::Module, ex::Any) @ HerbGrammar HerbGrammar.jl/src/csg/probabilistic_csg.jl:152 [5] include(fname::String) @ Base.MainInclude ./client.jl:489 [6] macro expansion @ HerbGrammar.jl/test/runtests.jl:6 [inlined] [7] macro expansion @ julia/stdlib/v1.10/Test/src/Test.jl:1577 [inlined] [8] top-level scope @ HerbGrammar.jl/test/runtests.jl:6 [9] include(fname::String) @ Base.MainInclude ./client.jl:489 [10] top-level scope @ none:6 [11] eval @ ./boot.jl:385 [inlined] [12] exec_options(opts::Base.JLOptions) @ Base ./client.jl:291 [13] _start() @ Base ./client.jl:552 in expression starting at HerbGrammar.jl/test/test_csg.jl:182 in expression starting at HerbGrammar.jl/test/test_csg.jl:1 Test Summary: | Error Total Time HerbGrammar.jl | 1 1 1.4s ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken. in expression starting at HerbGrammar.jl/test/runtests.jl:5 ERROR: Package HerbGrammar errored during testing ```

This fix removes the line numbers and fixes the faulty index that relied on the line numbers being present. It also makes the probabilistic macros return an Expr. This should have been included in #75, but I missed it before.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 45.95%. Comparing base (d3ff232) to head (c5105ad).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #84 +/- ## ========================================== + Coverage 45.15% 45.95% +0.79% ========================================== Files 8 8 Lines 454 457 +3 ========================================== + Hits 205 210 +5 + Misses 249 247 -2 ```

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