Test adequacy #215
Test adequacy #215
Conversation
…ngFramework into test-adequacy
…rk into test-adequacy
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports MegaLinter is graciously provided by |
…rk into test-adequacy
Codecov Report
@@ Coverage Diff @@
## main #215 +/- ##
==========================================
+ Coverage 95.44% 95.63% +0.19%
==========================================
Files 19 20 +1
Lines 1338 1399 +61
==========================================
+ Hits 1277 1338 +61
Misses 61 61
Continue to review full report in Codecov by Sentry.
|
| @@ -66,9 +67,8 @@ def set_paths(self, json_path: str, dag_path: str, data_paths: list[str] = None) | |||
| data_paths = [] | |||
| self.input_paths = JsonClassPaths(json_path=json_path, dag_path=dag_path, data_paths=data_paths) | |||
|
|
|||
| def setup(self, scenario: Scenario): | |||
| def setup(self, scenario: Scenario, data=None): | |||
There was a problem hiding this comment.
I can't see this data parameter used in any setup calls, is this for some future use?
There was a problem hiding this comment.
I am using it as part of my case study so I can pass in the data directly rather than having to pass in filepaths
| if "coverage" in test and test["coverage"]: | ||
| adequacy_metric = DataAdequacy(causal_test_case, estimation_model, self.data_collector) | ||
| adequacy_metric.measure_adequacy() | ||
| # self._append_to_file(f"KURTOSIS: {effect_estimate.mean()}", logging.INFO) |
There was a problem hiding this comment.
Should we keep these commented code lines?
There was a problem hiding this comment.
Probs not. I'll delete before I merge
| converted = [] | ||
| for r in results[field]: | ||
| if isinstance(r, float): | ||
| converted.append( |
There was a problem hiding this comment.
Does this leave you with a list of dataframes of length 1? If so it seems quite inefficient and convoluted.
Would making converted a blank dataframe and using the df.append method so it is only 1 dataframe work?
There was a problem hiding this comment.
I did this so it type checks. If you have a categorical variable, statsmodels handles this with a dummy encoding, so you get a dataframe of coefficients with one for each category. It's easier to turn everything to a df than to handle the two different datatypes separately, although that's probably just me thinking like a functional programmer again...
| "FAILING ON", | ||
| [(ci_low, ci_high) for ci_low, ci_high in zip(ci_low, ci_high) if not ci_low < 0 < ci_high], | ||
| ) | ||
| # if not all(ci_low < 0 < ci_high for ci_low, ci_high in zip(ci_low, ci_high)): |
There was a problem hiding this comment.
If avoidable I think it would be better to just remove code rather than commenting out. It can always be found in previous versions of the framework if needed again.
There was a problem hiding this comment.
Again, you're probably right. I'm just a lazy programmer.
| from causal_testing.specification.causal_specification import CausalSpecification | ||
|
|
||
|
|
||
| class TestJsonClass(unittest.TestCase): |
There was a problem hiding this comment.
test class name and docstring might need updating
No description provided.