Json poisson example #67

christopher-wild · 2022-05-18T14:41:40Z

I think a discussion around the usability of https://github.com/CITCOM-project/CausalTestingFramework/blob/json_poisson_example/json_frontend/examples/poisson/causal_test_setup.py would be useful.

Currently the frontend assumes you correctly structure the custom user python code as per the example which isn't ideal. The obvious alternative to me is having a class that can be extended, maybe even an abstract one with properties to enforce it is done correctly. However if we want the framework to be as open as possible to people without python background classes and class extensions might just be another barrier to using it?

Not sure if this is best discussed in a comment or two or with a meeting :)

jmafoster1 · 2022-05-18T14:44:21Z

Personally, I quite like a class extension, but it is potentially a "barrier to entry". Having said that, if someone can write a complex computational model which requires our testing framework in order to test it effectively, they should be able to manage extending a class...

christopher-wild · 2022-05-18T14:50:26Z

That is probably very true, if they can jump every barrier up to the point of using the framework, one class extension hopefully won't stop them!

I'll create the class extension and then if needed, we can always discuss again when it's in front of us

codecov · 2022-05-18T14:52:10Z

Codecov Report

Merging #67 (e29ebf0) into main (8637a88) will decrease coverage by 9.97%.
The diff coverage is 1.02%.

@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
- Coverage   86.95%   76.98%   -9.98%     
==========================================
  Files          12       13       +1     
  Lines         820      934     +114     
==========================================
+ Hits          713      719       +6     
- Misses        107      215     +108

Impacted Files	Coverage Δ
causal_testing/json_front/json_class.py	`0.00% <0.00%> (ø)`
causal_testing/testing/causal_test_outcome.py	`76.38% <ø> (+3.25%)`	⬆️
causal_testing/data_collection/data_collector.py	`84.37% <100.00%> (ø)`
causal_testing/testing/intervention.py	`0.00% <0.00%> (-100.00%)`	⬇️
causal_testing/testing/causal_test_case.py	`100.00% <0.00%> (ø)`
causal_testing/specification/causal_dag.py	`98.61% <0.00%> (+0.21%)`	⬆️
causal_testing/testing/causal_test_engine.py	`91.42% <0.00%> (+0.38%)`	⬆️

Continue to review full report at Codecov.

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

bobturneruk · 2022-05-25T12:26:04Z

How would it be if https://github.com/CITCOM-project/CausalTestingFramework/blob/json_poisson_example/json_frontend/process_causal_tests_json.py were moved into causal_testing/, https://github.com/CITCOM-project/CausalTestingFramework/tree/json_poisson_example/json_frontend/examples/poisson moved into examples/ and the readmes merged? Would this better present the "JSON Frontend" as part of causal testing framework? cc @jmafoster1 @cwild-UoS @AndrewC19

jmafoster1 · 2022-05-25T12:43:22Z

Go ahead. I have no strong opinions...

christopher-wild · 2022-05-26T08:56:43Z

How would it be if https://github.com/CITCOM-project/CausalTestingFramework/blob/json_poisson_example/json_frontend/process_causal_tests_json.py were moved into causal_testing/, https://github.com/CITCOM-project/CausalTestingFramework/tree/json_poisson_example/json_frontend/examples/poisson moved into examples/ and the readmes merged? Would this better present the "JSON Frontend" as part of causal testing framework? cc @jmafoster1 @cwild-UoS @AndrewC19

Sounds good to me @bobturneruk, it could be overlooked in its current structure

bobturneruk

Looks good. It would be great to see json_frontend/process_causal_tests_json.py brought into the package and the example moved into the folder with the other examples. Please ignore the codacy notes. I think codecov and github are having some interaction problems. Tests for json_frontend/process_causal_tests_json.py should be added later.

jmafoster1 · 2022-07-14T14:19:35Z

When I run it, I'm getting a bunch of errors of the form

TypeError: 'NoneType' object is not callable
Exception ignored in: <function AstRef.__del__ at 0x7fe9e8920280>
Traceback (most recent call last):
  File "/home/michael/anaconda3/envs/causecumber/lib/python3.9/site-packages/z3/z3.py", line 351, in __del__

Not a very helpful stack trace, but please can you look into this?

christopher-wild · 2022-07-14T15:06:17Z

Very unhelpful stack trace I agree..

I just tried again with a fresh conda environment with the latest CTF from main branch and it worked okay. When did you last update the CTF in your conda env?

jmafoster1 · 2022-07-14T15:07:06Z

Thursday

Changed the backslashes to forward slashes in filepaths

bobturneruk · 2022-07-20T14:42:42Z

causal_testing/json_front/README.md

@@ -0,0 +1,32 @@
+# JSON Causal Testing Framework Frontend


This file is going to be a bit tricky to find here in the package. It might be better merged with the main readme for now.

Would it be a good idea to move this readme to the readthedocs and point to it from the main readme?

That would also work. Examples / tutorials should be prominent.

bobturneruk · 2022-07-20T14:44:21Z

causal_testing/json_front/README.md

+4. Meta constraint functions
+5. Mapping JSON distributions, effects, and estimators to python objects
+
+`examples/poisson/causal_tests.json` is the JSON file that allows for the easy specification of multiple causal tests.


We'll need quite a bit more instruction for anyone not already familiar with the package to use this. I suggest making an issue to keep track of this. It could be that a jupyter notebook is is a good next step.

bobturneruk · 2022-07-20T14:53:29Z

examples/poisson/causal_test_setup.py

+        description="A script for parsing json config files for the Causal Testing Framework")
+    parser.add_argument("-f", help="if included, the script will stop if a test fails",
+                        action="store_true")
+    parser.add_argument("--directory_path", help="path to the json file containing the causal tests",


I'm not sure the default will work here, as it's supposed to be setting a folder / directory, not a specific file. And the filenames are hardcoded elsewhere (in causal_testing/json_front/json_class.py). Maybe get rid of the CLI arg and look for files in the same folder as the script for now? Or get rid of the default value? We may want to look at this again after merging, depending on expected user needs around file locations and naming. I would have thought that, at least, not everyone would want to call their data data.csv which I think is the current requirement.

You're right. I think now that the door has been opened in terms of the user writing more python code, the simplest approach is expect the user to specify the paths in causal_test_setup.py. This way the file names and locations are completely flexible and up to the user

bobturneruk

I think this looks good. I'd like to get a deeper understanding and maybe a simpler example / more documentation / a jupyter notebook implementation will help with this once merged. I wonder if some things might be named differently. Maybe "json interface" is better than "json frontend"? And perhaps causal_test_setup.py could be run_causal_tests.py?

Generally liking how all the example specific stuff is in examples/poisson now.

examples/poisson/causal_test_setup.py

bobturneruk · 2022-07-20T14:58:09Z

setup.py

 ]

 setup(
    name="causal_testing_framework",
    version="0.0.1",
    install_requires=requirements,
    packages=find_packages(),
+    entry_points={
+        'console_scripts': [
+            'run_json = causal_testing.json_frontend:main'


I like this, but is it used in any of the current examples?

Thanks for spotting this, I was using this method before we swapped the main function from being in json_class.py to causal_test_setup.py. Now that the main function is in a script that the user is free to put and call from where they want, I'm not sure if there is a way to have an entry point to the main function.

Ill remove these lines for now

christopher-wild added 3 commits May 16, 2022 13:14

Add files to json_frontend directory

2cf389c

Move structure and add directory_path arg

5fa5517

Add README.md for frontend

41f99cf

christopher-wild requested review from jmafoster1 and bobturneruk May 18, 2022 14:41

christopher-wild added 5 commits May 23, 2022 11:56

codacy cleanup + docstrings

41f3d15

codacy import

bedf17d

fix typing

d3a332d

final attempt at import order

f84f547

add set comprehension

f97df0c

bobturneruk marked this pull request as ready for review May 25, 2022 12:15

bobturneruk approved these changes May 26, 2022

View reviewed changes

christopher-wild added 12 commits June 8, 2022 09:21

Move directory and add as CLI tool

0d20cf8

init json_class and new directory

bad0a2f

populate and structure json abc class

2347bb5

expose estimation_model and add blank add_modelling_assumption method

1f5696d

move example files

3ef09b8

move user example script and adapt to class

7a59b3e

comments

6cf3bbf

cleanup imports

0c968d9

cleanup test_case execution

f3220d1

private appropriate methods

c730d6f

add docstrings

64fa9e3

bug fix + docstring

b5972b4

christopher-wild added 7 commits June 30, 2022 11:19

cleanup import

5d5b8df

remove 'Variables' from the json file

ea9e5b0

remove print

1110d6f

Move and update README.md

0b91c5e

Add Comments

7c8a3f9

Codacy cleanup

0900f47

Move mutates dictionary

bf8b67a

jmafoster1 and others added 2 commits July 14, 2022 16:24

Fixed the "all the tests are failing" bug

9f9fd02

Update README.md

fd070db

Changed the backslashes to forward slashes in filepaths

bobturneruk reviewed Jul 20, 2022

View reviewed changes

bobturneruk approved these changes Jul 20, 2022

View reviewed changes

christopher-wild added 5 commits July 20, 2022 17:31

change path method

f5871b9

update README.md

7f4893c

Remove unused endpoints

84f43a4

update docs and rename files

bfeed04

remove unused import

e29ebf0

christopher-wild merged commit 4a6958b into main Jul 21, 2022

christopher-wild deleted the json_poisson_example branch July 21, 2022 15:28

Json poisson example #67

Json poisson example #67

Uh oh!

Conversation

christopher-wild commented May 18, 2022

Uh oh!

jmafoster1 commented May 18, 2022

Uh oh!

christopher-wild commented May 18, 2022

Uh oh!

codecov bot commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bobturneruk commented May 25, 2022

Uh oh!

jmafoster1 commented May 25, 2022

Uh oh!

christopher-wild commented May 26, 2022

Uh oh!

bobturneruk left a comment

Choose a reason for hiding this comment

Uh oh!

jmafoster1 commented Jul 14, 2022

Uh oh!

christopher-wild commented Jul 14, 2022

Uh oh!

jmafoster1 commented Jul 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobturneruk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented May 18, 2022 •

edited

Loading