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

add un age dist #29

Merged
merged 3 commits into from
Apr 12, 2020
Merged

add un age dist #29

merged 3 commits into from
Apr 12, 2020

Conversation

mathijshenquet
Copy link

No description provided.

epimodel/exports/epidemics_org.py Show resolved Hide resolved
epimodel/exports/epidemics_org.py Outdated Show resolved Hide resolved
epimodel/exports/epidemics_org.py Outdated Show resolved Hide resolved
d["Timezones"] = timezones["Timezone"].tolist()

if un_age_dist is not None:
d["AgeDist"] = un_age_dist.iloc[:-3].to_dict()
Copy link

Choose a reason for hiding this comment

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

If I am looking correctly at this, it means all columns but 95-99 | 100+ ? So the population is not enough, you want to have it by age for future use on frontend for various purposes?

Also, please use explicit column names, pandas is clever and can do e.g. un_age_dist.loc[:"85-89"]. Fighting bugs with number-indexed columns is crazy.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that was just the quickest thing I could come up with, you probably know a better way: I just wanted to get rid of the columns "Type","Region Name","Parent Code M49".

Copy link

Choose a reason for hiding this comment

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

Btw. I think I could get the true population as we had it in the past under 30 minutes for all countries we export.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry I was unclear. What you wrote above was not my intent but rather I wanted to remove just the columns "Type","Region Name","Parent Code M49".

Copy link
Author

Choose a reason for hiding this comment

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

Yes I think having the age distribution might be nice because we could use it in the future the calculate the CFR in some other way as those in rates.csv.

Copy link

@hnykda hnykda Apr 12, 2020

Choose a reason for hiding this comment

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

Sorry I was unclear. What you wrote above was not my intent but rather I wanted to remove just the columns "Type","Region Name","Parent Code M49".

Then do un_age_dist.drop(columns=["Type","Region Name","Parent"]) to be explicit.

Yes I think having the age distribution might be nice because we could use it in the future the calculate the CFR in some other way as those in rates.csv.

OK, so keeping as is if you are fine with that?

Co-Authored-By: Daniel Hnyk <hnykda@users.noreply.github.com>
@hnykda
Copy link

hnykda commented Apr 12, 2020

Can we merge this into #25 already?

@mathijshenquet
Copy link
Author

I fixed the un_age_dist.drop(columns=["Type","Region Name","Parent"]) thing so should be mergable.

@hnykda hnykda merged commit 32be566 into daniel-pipe-completion Apr 12, 2020
@hnykda hnykda deleted the mathijs-pipe-completion branch April 12, 2020 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants