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

Move Age Variable Counts to SAS Scripts #183

Merged
merged 3 commits into from
Apr 26, 2018

Conversation

andersonfrailey
Copy link
Collaborator

This PR moves the creation of nu13, nu05, n24, elderly_dependent, f2441, and EIC for the CPS file from finalprep.py to the SAS scripts. I'm doing this because final prep was only able to count up to five dependents for each of these variables, while in the SAS scripts we should be able to catch all of them.

It is labeled WIP because n24 exceeds nu18 in about 7% of the records still. I'll remove the label when I figure out why this is the case. I also incremented EIC incorrectly for 2013 and 2014, which I'll fix in my next commit.

@andersonfrailey
Copy link
Collaborator Author

I've added some checks in the SAS scripts to ensure that n24 will not exceed nu18 now. n24 is now less than or equal to nu18 in each record in the CPS.

@martinholmer
Copy link
Contributor

@andersonfrailey said:

I've added some checks in the SAS scripts to ensure that n24 will not exceed nu18 now. n24 is now less than or equal to nu18 in each record in the CPS.

That's great! Thanks for the quick resolution of this issue.
Why don't you removed the WIP part of the title of PR #183 and merge it.

@andersonfrailey andersonfrailey changed the title WIP: Move Age Variable Counts to SAS Scripts Move Age Variable Counts to SAS Scripts Apr 26, 2018
@martinholmer
Copy link
Contributor

@andersonfrailey, Do you think the changes in pull request #183 will resolve issue #149?

@andersonfrailey
Copy link
Collaborator Author

@martinholmer, the issue in #149 was resolved in PR #151 for the CPS file. I'm still working on a fix for the PUF. This PR was aimed at resolving just #157.

@martinholmer
Copy link
Contributor

@andersonfrailey said:

The issue in #149 was resolved in PR #151 for the CPS file. I'm still working on a fix for the PUF. This PR #183 was aimed at resolving just #157.

OK. Thanks for the clarification.

@andersonfrailey
Copy link
Collaborator Author

Updated this PR now that #184 has been merged. Ready to go upon review.

@martinholmer
Copy link
Contributor

@andersonfrailey said:

Updated this PR #183 now that #184 has been merged. Ready to go upon review.

I'm not conversant with SAS. Are you going to ask @Amy-Xu to review #183?

@andersonfrailey
Copy link
Collaborator Author

@Amy-Xu can review the code and anyone should be able to download the individual CSV files to look through if they so choose.

@andersonfrailey
Copy link
Collaborator Author

If there are no additional comments I'll merge this at the end of the work day.

@andersonfrailey andersonfrailey merged commit 6ac4f63 into PSLmodels:master Apr 26, 2018
@andersonfrailey andersonfrailey deleted the cpsagefixes branch April 26, 2018 21:08
@talumbau talumbau removed the ready label Apr 26, 2018
@Amy-Xu
Copy link
Member

Amy-Xu commented Apr 26, 2018

Sorry for the delay. Looks good to me.

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.

4 participants