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 pyrho genetic maps #572

Merged
merged 2 commits into from
Aug 1, 2020
Merged

Conversation

jeffspence
Copy link
Contributor

I added genetic maps for Homo sapiens for the 26 populations in Phase 3 of the 1000 Genomes Project from my paper here: https://advances.sciencemag.org/content/5/10/eaaw9206

I also alphabetized the references at the top of stdpopsim/catalog/HomSap/__init__.py

I'd be happy to make any additional changes (e.g., are genetic maps unit tested?).

-Added genetic maps for the 26 populations from Phase 3 of the 1000 Genomes Project that were inferred using pyrho for Spence and Song 2019.

-Alphabetized citations.
@andrewkern
Copy link
Member

thanks for the PR @jeffspence. it looks like one thing that is breaking the tests currently is the URL-- would you mind putting this on a publicly accessible address for a minute so that the PR can pass tests?

@ndukler do we have a SOP on how to deal with map downloading / caching before we merge?

file_pattern=(
"Pyrho{}_GRCh37_chr{{id}}.txt".format(pop)),
citations=[
stdpopsim.Citation(
Copy link
Member

@andrewkern andrewkern Jul 31, 2020

Choose a reason for hiding this comment

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

maybe pull this citation out of here and set it to _spence_song_2019 as with the other citations?

@ndukler
Copy link
Contributor

ndukler commented Jul 31, 2020

thanks for the PR @jeffspence. it looks like one thing that is breaking the tests currently is the URL-- would you mind putting this on a publicly accessible address for a minute so that the PR can pass tests?

@ndukler do we have a SOP on how to deal with map downloading / caching before we merge?

I haven't actually uploaded the maps yet so I expect the tests to fail. I just asked @jeffspence to throw up the PR so I could take a look through and make sure everything was in order. It looks pretty good so I'll upload the maps now.

@andrewkern
Copy link
Member

it would be great to have a sandbox to stage these things in during development. at the start we were just hosting them locally on various servers, but since we moved to AWS it's a bit harder

@ndukler
Copy link
Contributor

ndukler commented Aug 1, 2020

it would be great to have a sandbox to stage these things in during development. at the start we were just hosting them locally on various servers, but since we moved to AWS it's a bit harder

I agree, but it's fairly easy to just throw things up on AWS, even this only took a half hour or so and this was absolutely massive by our standards. In fact, we can only do something of this size twice more before we run out of our 2GB allowance :). Which is really my way of saying nice work @jeffspence , this is an awesome resource for us to have. Anyway, it's all up now so I just re triggered the travis build so lets see if it works.

@ndukler
Copy link
Contributor

ndukler commented Aug 1, 2020

Just as a record, I think I was wrong, the files do have to be flat in the tar.gz archive. I fixed it and will test. If that's the case I'll update the devdocs to make sure this bit of information doesn't have to be rediscovered

@ndukler
Copy link
Contributor

ndukler commented Aug 1, 2020

Ok, so things look good! I cannot trigger a manual rebuild for CircleCI or appveyor so add a comment somewhere, rebase and force push to get it going again @jeffspence .

@codecov
Copy link

codecov bot commented Aug 1, 2020

Codecov Report

Merging #572 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #572   +/-   ##
=======================================
  Coverage   99.64%   99.65%           
=======================================
  Files          29       29           
  Lines        1997     2000    +3     
  Branches      206      207    +1     
=======================================
+ Hits         1990     1993    +3     
  Misses          3        3           
  Partials        4        4           
Impacted Files Coverage Δ
stdpopsim/catalog/HomSap/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e148606...d0f0596. Read the comment docs.

@andrewkern andrewkern merged commit dc8315d into popsim-consortium:master Aug 1, 2020
@jeffspence
Copy link
Contributor Author

jeffspence commented Aug 1, 2020

Thanks @ndukler and @andrewkern ! It was a real pleasure!

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.

None yet

3 participants