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

fixed warnings and progrss bar improvements #567

Merged
merged 6 commits into from Aug 19, 2022

Conversation

yemaedahrav
Copy link
Contributor

Fixed the warnings arising in propensity score estimators and added optional progress bars for refuters

Signed-off-by: Amey Varhade ameyvarhade@gmail.com

Fixed the warnings arising in propensity score estimators and added optional progress bars for refuters

Signed-off-by: Amey Varhade <ameyvarhade@gmail.com>
@yemaedahrav
Copy link
Contributor Author

The progress bars have been added; the default is set to False, and the verbose mode from joblib provides updates, but alternatively, this can be used. The warning arising in propensity score estimators, as seen here, is fixed.

Copy link
Member

@amit-sharma amit-sharma left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @yemaedahrav ! Added a few comments. Once you address them, I can merge.

dowhy/causal_refuters/add_unobserved_common_cause.py Outdated Show resolved Hide resolved
docs/source/example_notebooks/dowhy_simple_example.ipynb Outdated Show resolved Hide resolved
dowhy/causal_refuters/data_subset_refuter.py Outdated Show resolved Hide resolved
yemaedahrav and others added 2 commits July 28, 2022 22:26
Signed-off-by: Amey Varhade <ameyvarhade@gmail.com>
@yemaedahrav
Copy link
Contributor Author

The changes are in the commit. I also noticed in the build logs that it failed in the dowhy_demo_dummy_outcome_refuter notebook however, I have not made any modifications to the dummy outcome refuter and in the latest release version it works perfectly fine.

@amit-sharma
Copy link
Member

That's okay. Can you push the commit to this branch yemaedahrav:amey/progress-warnings so it becomes a part of the PR?

@yemaedahrav
Copy link
Contributor Author

Now, this PR has all the relevant commits

@amit-sharma
Copy link
Member

@yemaedahrav there is a bug in this PR. Can you check?
File ~/work/dowhy/dowhy/dowhy/causal_model.py:422, in CausalModel.refute_estimate(self, estimand, estimate, method_name, show_progress_bar, **kwargs)
414 refuter_class = causal_refuters.get_class_object(method_name)
416 refuter = refuter_class(
417 self._data,
418 identified_estimand=estimand,
419 estimate=estimate,
420 **kwargs
421 )
--> 422 res = refuter.refute_estimate(show_progress_bar)
423 return res

TypeError: refute_estimate() takes 1 positional argument but 2 were given
TypeError: refute_estimate() takes 1 positional argument but 2 were given

Copy link
Member

@amit-sharma amit-sharma left a comment

Choose a reason for hiding this comment

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

Thanks @yemaedahrav . This is a very useful addition to track the progress of refutation tests.

@amit-sharma
Copy link
Member

@all-contributors please add @yemaedahrav for code.

@allcontributors
Copy link
Contributor

@amit-sharma

I've put up a pull request to add @yemaedahrav! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants