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

210 review helpfile #213

Merged
merged 13 commits into from
May 23, 2024
Merged

210 review helpfile #213

merged 13 commits into from
May 23, 2024

Conversation

jorpppp
Copy link
Collaborator

@jorpppp jorpppp commented May 20, 2024

Closes #210

@jorpppp jorpppp linked an issue May 20, 2024 that may be closed by this pull request
@jorpppp
Copy link
Collaborator Author

jorpppp commented May 20, 2024

@Constantino-Carreto-Romero can you please read these helpfiles again to see if everything is clear and if you spot any mistakes?


{marker imputetype}{...}
{phang}
{opt impute(type, [ saveimp])} imputes missing values in {it:policyvar} and uses this new variable as the actual {it:policyvar}.
{opt impute(type, [ saveimp])} imputes leads, lags, and missing values of {it:policyvar} and uses this new variable as the actual {it:policyvar}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jorpppp
What do you think of adding here in the impute section a suggestion for the user to check the impute note?

Copy link
Collaborator Author

@jorpppp jorpppp May 20, 2024

Choose a reason for hiding this comment

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

Good idea, can you implement please @Constantino-Carreto-Romero ?

Also please edit the impute note to make the language consistent: "imputes leads, lags and missing values of the policy variable".

Copy link
Collaborator

Choose a reason for hiding this comment

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

implemented in d293178

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, don't forget to edit the impute note too (we should do it in this PR).

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jorpppp
I updated the impute note in 2463388. I added an index at the beginning of the note as well. The updated note looks like this. I uploaded the files to create the impute note to an issue folder in 05f4489, just to have a commit to back them up.

@@ -293,20 +314,17 @@ the cohort variable. {opt save} adds the new control cohort variable to the data
is not specified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

A minor clarification, I think we could change it to:
"{opt control_cohort(create)} is the default if {opt cohort(create)} is specified but {opt control_cohort} is not specified."

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, agreed, please implement @Constantino-Carreto-Romero

Copy link
Collaborator

Choose a reason for hiding this comment

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

changed in ebb8939

@Constantino-Carreto-Romero
Copy link
Collaborator

@Constantino-Carreto-Romero can you please read these helpfiles again to see if everything is clear and if you spot any mistakes?

@jorpppp
Below there is a list of the issues covered since the last release. I corroborated that the help files incorporated the corresponding changes. I didn't find any missing changes in the help files, and all the incorporations looked clear to me.

@jorpppp
Copy link
Collaborator Author

jorpppp commented May 23, 2024

Thanks @Constantino-Carreto-Romero . I deleted the issue folder, merging now.

@jorpppp jorpppp merged commit 246aaf8 into main May 23, 2024
@jorpppp jorpppp deleted the 210-review-helpfile branch May 23, 2024 22:31
@jorpppp jorpppp mentioned this pull request May 23, 2024
jorpppp added a commit that referenced this pull request May 23, 2024
* #210 Changes to xtevent helpfile, enable abbreviations

* #210 Changes to other helpfiles

* #214 Change ci default style to p1 (#215) (#216)

* #210 add link to impute note

* #210 minor edit

* #210 minor edit to acknowledgements

* #210 minor clarification

* #210 update description of impute

* "#210 revert previous commit"

This reverts commit 824b616.

* #210 update impute note

* #210 revert minor unintended changes

* #210 docs to create impute note

* #210 #213 Delete issue folder

---------

Co-authored-by: Constantino Carreto Romero <carretoromeroc@gmail.com>
jorpppp added a commit that referenced this pull request May 30, 2024
* #217 Test adding more results to repost

* 210 review helpfile (#213) (#218)

* #210 Changes to xtevent helpfile, enable abbreviations

* #210 Changes to other helpfiles

* #214 Change ci default style to p1 (#215) (#216)

* #210 add link to impute note

* #210 minor edit

* #210 minor edit to acknowledgements

* #210 minor clarification

* #210 update description of impute

* "#210 revert previous commit"

This reverts commit 824b616.

* #210 update impute note

* #210 revert minor unintended changes

* #210 docs to create impute note

* #210 #213 Delete issue folder

---------

Co-authored-by: Constantino Carreto Romero <carretoromeroc@gmail.com>

* #217 post est results from interacted regression

* #217 original test file output

* #217 test file output after changes to eventols

* #217 SA and trend adjustment by GMM

* #217 test file after edits to eventols

* #217 return b and V from the interacted regression

* #217 add to help about b and V from interacted reg

* #217 updated log file

---------

Co-authored-by: Constantino Carreto Romero <carretoromeroc@gmail.com>
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.

Review helpfile
2 participants