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

feat: test_itr & website & summary() for test_itr outputs #37

Merged
merged 32 commits into from
May 1, 2023

Conversation

xiaolong-y
Copy link
Collaborator

@xiaolong-y xiaolong-y commented Apr 24, 2023

  • complete version of test_itr
  • website with 'pkgdown': edited website home page, more vignettes to be written
  • summary statistics for test_itr outputs

xiaolong-y and others added 30 commits December 31, 2022 10:19
This reverts commit 384085b.
- models trained with caret seems slightly problematic
- run_itr to estimate_itr
- estimate_itr to evaluate_itr
- integrate both consistency and heterogeneity tests to the main workflow (under main.r)
- transformation function to obtain the tau_cv matrix
- single outcome, single model only  for this commit
- (so great to have solved a lingering bug!!)
- improve gettaucv in helper function to extract k folds of tau_cv in every algorithm j
- fix the cross validation part of test_itr
Add
- intro
- tutorial
- articles

Edit main page
- takes output directly from `test_itr()` and prints tibble (print.`summary.itr()` function)
@xiaolong-y
Copy link
Collaborator Author

Current problems with summary()for test_itr outputs:

  • while summary.test.itr() prints summary statistics,summary() does not
  • summary statistics only show output structure (ex: <dbl [1]>), not the actual output

@xiaolong-y xiaolong-y changed the title feat: test_itr & website feat: test_itr & website & summary() for test_itr outputs Apr 24, 2023
@xiaolong-y xiaolong-y requested a review from jialul April 24, 2023 09:46
@jialul
Copy link
Collaborator

jialul commented Apr 27, 2023

@xiaolong-y you mentioned that you have further worked on it to resolve the issue this Monday. Has the summary function issue been resolved? Or do you want me to look into your code?

R/data 2.R Outdated
Copy link
Collaborator

@jialul jialul Apr 27, 2023

Choose a reason for hiding this comment

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

The data.R is already included in the R folder, please do not add duplicated files to it. While pushing new files to GitHub, I believe you can select the new updates while keeping all the temporary files in your local directories.

README.Rmd Show resolved Hide resolved
Copy link
Collaborator

@jialul jialul left a comment

Choose a reason for hiding this comment

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

Hi Xiaolong, I noticed that your current workflow is (1) pull from my causal-ml branch; (2) work on project on your xiaolong-y:causal-ml branch; (3) push your branch to MichaelLLi:causal-ml-gates.

However, when you push the changes to MichaelLLi:causal-ml-gates, this workflow would lead to your updates intertwined with my changes. This makes it really hard for me to see what are the new changes you made since they are mixed up with other updates I made to MichaelLLi:causal-ml.

Moving forward, I suggest using the following workflow: (1) make sure everything is up-to-date on MichaelLLi:causal-ml-gates; (2) work on project locally and push to MichaelLLi:causal-ml-gates; (3) if you want me to take a look at your code, request review on MichaelLLi:causal-ml-gates and I'll check it.

man/compute_qoi 2.Rd Outdated Show resolved Hide resolved
@xiaolong-y xiaolong-y merged commit f259fd2 into MichaelLLi:causal-ml-gates May 1, 2023
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.

2 participants