Jmafoster1/scenario

[jmafoster1]

This PR removes the Scenario class and merges what we need into the CausalSpecification class.

I appreciate that this sounds drastic, but I realised that we weren't testing any of the Scenario class methods. When I investigated, I realised that they were all just infrastructure for Z3, which we stripped out a while ago. Thus, it makes much more sense in my opinion to merge what we do actually need from Scenario into the CausalSpecification class.

[github-actions]

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ PYTHON	black	32		0	0.98s
✅ PYTHON	pylint	32		0	5.8s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.22%. Comparing base (30a4250) to head (4c91533).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #370      +/-   ##
==========================================
+ Coverage   96.64%   97.22%   +0.58%     
==========================================
  Files          28       27       -1     
  Lines        1608     1551      -57     
==========================================
- Hits         1554     1508      -46     
+ Misses         54       43      -11     

Files with missing lines	Coverage Δ
causal_testing/main.py	100.00% <100.00%> (ø)
causal_testing/specification/causal_dag.py	98.25% <100.00%> (-0.64%)	⬇️
...usal_testing/specification/causal_specification.py	93.75% <100.00%> (+2.84%)	⬆️
...sal_testing/surrogate/causal_surrogate_assisted.py	100.00% <100.00%> (ø)
...l_testing/surrogate/surrogate_search_algorithms.py	98.50% <100.00%> (ø)
causal_testing/testing/metamorphic_relation.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3d2915...4c91533. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[f-allian]

@jmafoster1 This is quite a big change, but it is something I raised several months ago. My main point was that the casual specification class is programmatically redundant (even more so now that you're suggesting removing the modelling scenario class). But, I now think that we need to stay loyal to the original idea of the framework to some degree. Probably best talking about this in our meeting tomorrow before making any more changes.

[jmafoster1]

But, I now think that we need to stay loyal to the original idea of the framework to some degree.

Philosophically, I reject this on the basis that the framework has grown and changed so much from the original idea, but I think your point about the CausalSpecification class being redundant is very valid. It served us well as a packer for the Scenario and CausalDAG, but there were only a few remaining classes that still used it. It only took me half an hour to strip it out, so I submitted #371 so we can compare the two approaches in the meeting later and close the PR we don't go with. Either way, we'll still need to fix the IPYNB tutorials and cover the vaccinating the elderly example.

As a sidenote, is there a way to make running the tutorials part of the PR actions so they don't become out of date?

[jmafoster1]

Following on from our meeting this morning, I'll close this one in favour of #371, which is more modular and faithful to the original vision for the CTF.