Some clean up of contexts #711
Conversation
it's no longer used functionality (was dropped when we dropped the logprob-macro)
Pull Request Test Coverage Report for Build 11707808796Details
💛 - Coveralls |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 82.14% 83.65% +1.51%
==========================================
Files 30 30
Lines 4200 4166 -34
==========================================
+ Hits 3450 3485 +35
+ Misses 750 681 -69 ☔ View full report in Codecov by Sentry. |
|
Thanks @penelopeysm !
I think we need a way to test this though (as that would have caught the current bug). IMO every context should at be evaluated with both |
…placed much of the `test/contexts.jl` with calls to this method
|
Think this PR is ready for another round of reviews 👍 Added proper testing for the contexts + a |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
| @testset "setchildcontext" begin | ||
| @testset "nested contexts" begin | ||
| # Both of the following should result in the same context. | ||
| context1 = ParentContext(ParentContext(ParentContext())) | ||
| context2 = setchildcontext( | ||
| ParentContext(), setchildcontext(ParentContext(), ParentContext()) | ||
| ) | ||
| @test context1 === context2 | ||
| end | ||
|
|
||
| @testset "$context" for context in parent_contexts | ||
| # Setting the child context to a leaf should now change the `leafcontext` accordingly. | ||
| new_leaf = | ||
| leafcontext(context) isa DefaultContext ? PriorContext() : DefaultContext() | ||
| new_context = setchildcontext(context, new_leaf) | ||
| @test childcontext(new_context) === leafcontext(new_context) === new_leaf |
There was a problem hiding this comment.
Did these tests get cut for a reason? As I don't see corresponding ones in the new test_context.
Some cleanup of the context implementations:
PrefixContextwhich doesn't actually work for.~statements due to implementing the incorrect signature.tilde_*_assume/observemethods forPriorContextandLikelihoodContext+ removed theirvarsfield which is also not used any more (this was only used by the logprob macro which was removed in Remove theprobandlogprobmacros #604 )