Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: docs revamp #249

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Update: docs revamp #249

merged 3 commits into from
Dec 6, 2023

Conversation

f-allian
Copy link
Contributor

Closes #242

@f-allian f-allian added the documentation Improvements or additions to documentation label Nov 16, 2023
@f-allian f-allian self-assigned this Nov 16, 2023
Copy link

github-actions bot commented Nov 16, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 26 0 0.89s
✅ PYTHON pylint 26 0 3.94s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link

codecov bot commented Nov 16, 2023

Codecov Report

Merging #249 (e997aac) into main (b092db2) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #249   +/-   ##
=======================================
  Coverage   95.32%   95.32%           
=======================================
  Files          20       20           
  Lines        1411     1411           
=======================================
  Hits         1345     1345           
  Misses         66       66           

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 a363dca...e997aac. Read the comment docs.

@christopher-wild
Copy link
Contributor

Apologies for not reviewing this sooner!

I've just tried to build it locally and get the following errors:

WARNING: html_static_path entry '_static' does not exist
WARNING: logo file '_static/images/CITCOM-logo.png' does not exist

Looking at the commits these are not yet added to git

@christopher-wild
Copy link
Contributor

Small thing but I also get this glossary warning

C:\Users\cs1xcw\PycharmProjects\CausalTestingFramework\docs\source\index.rst:9: WARNING: term not in glossary: 'directed acyclic graphs'

Not sure if this is a problem or not though

Copy link
Contributor

@christopher-wild christopher-wild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add my comments as a review so I'll add this. Like we discussed though no rush on this at all. It just means when you push the changes you can re-request a review and I'll get pinged :)

@f-allian
Copy link
Contributor Author

f-allian commented Dec 6, 2023

Forgot to add my comments as a review so I'll add this. Like we discussed though no rush on this at all. It just means when you push the changes you can re-request a review and I'll get pinged :)

Thanks for the feedback @cwild-UoS. That should be all fixed now - you'll likely still receive the usual autoapi warnings, though.

Copy link
Contributor

@christopher-wild christopher-wild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Cheers for doing this Farhad

Delta is an intervention which should be applied to X, and Y is the expected *causal effect* of that intervention on
some output of interest. Therefore, a causal test case states the expected causal effect (Y) of a particular
intervention (Delta) made to an input configuration (X). For each scenario, the user should create a suite of causal
a causal test case is a triple ``(M, X, Delta, Y)``, where ``M`` is the modelling scenario, ``X`` is an input configuration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess causal test cases are no longer technically "triple"s but I don't think this matters

@f-allian f-allian merged commit 44daba4 into main Dec 6, 2023
6 checks passed
@f-allian f-allian linked an issue Mar 27, 2024 that may be closed by this pull request
@f-allian f-allian mentioned this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update and Improve the Documentation Glossary
2 participants