-
Notifications
You must be signed in to change notification settings - Fork 2
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
Adds popshare option #19
Conversation
Web Scrapping coverage + draft clean up
Censors regional estimates given coverage
Set values to missing instead of deleting.
draft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HI David,
I think the code looks really good. There is one important comment regarding a noi
instead of a oi
, but the other comment is just about logical coherence. The function works, but the conditions do not exclude each other.
Thanks you
povcalnet.ado
Outdated
*---------- Poverty line/population share | ||
if ("`popshare'" != ""){ | ||
if ("`povline'" != ""){ | ||
oi disp as err "povline and popshare cannot be used at the same time" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I Think this should be noi
rather than oi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awful typo, I'll fix it right away
povcalnet.ado
Outdated
loc pcall = "popshare" | ||
} | ||
} | ||
else if ("`povline'" == "") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is not mutually exclusive with the first condition and with the else statement. I think the problem is that these lines,
if ("`popshare'" != ""){
if ("`povline'" != ""){
Should be just one line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, I'll adjust it
Hi Davdi, one more thing. It is still not working with several values in popshare. For instance, it is not working like this, povcalnet, popshare(.2 .5) clear
povcalnet, popshare(.2 .5) clear fillgaps However, I am not worry about it. We could create a loop and make the query. It is not a big deal. Thanks. |
draft
Hi Andres, All fixed and rebased to ease the merge. Let me know if something is missing. Thanks ! |
Perfect. All changes have been added. |
No description provided.